Re: mailgw argument parsing

"John P. Rouillard" <[email protected]> Wed, 23 Nov 2022 10:58:02 -0500
Newsgroups gmane.comp.bug-tracking.roundup.devel
Message-ID <[email protected]>
Hi Ralf:

In message <[email protected]>,
Ralf Schlatterbeck writes:
>On Tue, Nov 22, 2022 at 03:20:05PM -0500, John P. Rouillard wrote:
>> But I really don't have a leg to stand on anymore since the program
>> documents multiple [-C]/-S segments are valid.
>> 
>> So I withdraw my objection. Feel free to update the docs (include the
>> man page please) and merge. I agree once you get more than one set of
>> -C/-S pairs in place, your -S class.prop=value is much cleaner.
>
>Thanks!
>
>Note that I think the feature is rarely used anyway.

It may be, but I know at least one person (besides the trackers I have
set up) who was using it simiarly to me for multiple levels of
discussion on a ticket. Also another one used it for permissions on a
security reporting address.

>I've also fixed the classname case, so if you have -c to set the default
>class, you can now use the same classname in
>-S classname.prop=value
>the original implementation required that you used 'issue' even if your
>issue-class was named differently. And I'll allow *several* issue
>classes (for the case where the class is mentioned in the subject of a
>reply).

Nice. Having those codepaths tested is great too.

>And during testing I've discovered that the old code would try to set
>the defined attributes on the class given in the message (the 'issue'
>class) if you have something like an email with the subject
>
>Subject: [keyword23] this is a test of setting keyword parameters
>
>The call to get_class_arguments has class_type='issue' and
>classname='keyword' and would try to apply the -S options for issue to
>keyword. We do have unittests for this case with keyword in the subject
>(but without -S options)!

Hence the test passed. Now we can test with -S and still have it pass
8-).

>My current implementation ignores these calls when the classname does
>not match.

Do you think it should log instead? Something like:

   ignoring -S option foo.private=true as message is for class keyword

Even though -S is used rarely, I can see debugging this being an issue.
Also what happens if you have to issue classes:

  bug and feature

with the same set of properties. If a subject [bug12345] comes in and
you have:

  -S feature.status=low

it should be ignored. Right?? If you have

  -S status=low

what happens? Applied??

This could lead to a lot of logging. Maybe a flag to enable debug
logging and put these "ignoring -S option foo...." options behind it?
Thoughts?

Also it looks like the 'Mail gateway script command line' is
documented in user-guide.txt. Any idea why? Maybe after your changes
we can move the revised command line/setup info into admin-guide. This
still leaves more technical mailgw stuff in the user guide but is a
start to cleaning it up.

Have a great rest of the week.

--
				-- rouilj
John Rouillard
===========================================================================
My employers don't acknowledge my existence much less my opinions.