Hi Colin,
I must ashamedly admit that I haven't kept up with debbugs development
since DebConf 3. I guess I have to catch up now. I will do my best...
Am Mi, den 04.02.2004 schrieb Colin Watson um 00:17:
> Subscribers should only get one of "acknowledged by developer" or
> "marked as done", not both. The PTS gets "marked as done", but actually
> I think in this case it might be better to send subscribers
> "acknowledged by developer", since they tend to have a more user-like
> role than people who subscribe to a whole package.
Agreed.
> It seems that it would sometimes make sense for the Reply-To: header to
> be changed. For instance, if $replyto is already subscribed to the bug,
> or is the submitter, then you don't want to direct people to cc that
> person again.
Sounds good. Who else should not be put there? The package maintainer?
We would have to compare e-mail-adresses. Should we use a module for
mormalistion or comparisation, or just use some simple regex hacks? What
modules may we use, BTW - only from stable, or also ones that are only
in unstable?
> This is the -maintonly/-quiet case, and subscribers shouldn't receive
> messages from here.
Agreed
> I think we can just put the subscribers into @$bcc, really. If they turn
> out to need special handling later than we can separate them out then,
> but for now I'd just keep the interface simple.
My original motivation was that I might come across a case where
sendmessage is called with nothing (so that "-t" kicks in), but where I
would have to add subscribers as recipients. I am not sure if that was
the case. Maybe the auto-"-t"-feature should be dropped and the
recipient should be specified explicitly, or we fill in all addresses in
"To:" and "Bcc:" headers, but not rely on this mixture.
> We ought to think about some way to make sure that people don't get too
> many duplicate messages.
Reasonable to just compare each with every recipient, dropping all
duplications, keeping the one in "To:", if the other one is in "Bcc:".
Could reuse the code that checks if the Reply-To: is subscribed.
> > +sub subfile_load {
> > + # Loads all subscriptions from the file into a hashref
> > + my $file = shift;
> > + my $data = {};
> > + open SUBFILE,"<$file" || &quit("open $file: $!");
> > + while (<SUBFILE>) {
> > + m/^([a-z0-9]+)\s+(.*)$/;
>
> Should check whether this match succeeded.
Good Point. I will check all my code for places where more
error-checking is due.
> I think I'd prefer the e-mail address first, too.
I put it this way, because the hash or "confirmed" is sure and easy to
match, and I can use (.*) for the e-mail address (and the match would be
faster *g*). Why would you prefer it the other way?
> > +sub bugfile {
> That's getbugcomponent().
Oh, I should have seen that...
> subfile_load is common with service.in, so instead of duplicating code
> this should live in errorlib.in.
Agreed.
> > diff -ru source-alt/scripts/service.in source/scripts/service.in
> > --- source-alt/scripts/service.in 2003-06-23 13:23:35.000000000 +0200
> > +++ source/scripts/service.in 2003-07-16 09:23:32.000000000 +0200
> [...]
> > @@ -214,18 +215,58 @@
> > $ok++;
> > } elsif (m/^refcard/i) {
> > &sendtxthelp("bug-mailserver-refcard.txt","mail servers' reference card");
> > - } elsif (m/^subscribe/i) {
> > - &transcript(<<END);
> > -There is no $gProject $gBug mailing list. If you wish to review bug reports
> > -please do so via http://$gWebDomain/ or ask this mail server
> > -to send them to you.
> > -soon: MAILINGLISTS_TEXT
> > -END
> > - } elsif (m/^unsubscribe/i) {
> > - &transcript(<<END);
> > -soon: UNSUBSCRIBE_TEXT
> > -soon: MAILINGLISTS_TEXT
> > -END
> > + } elsif (m/^subscribe\s+\#?(-?\d+)(?:\s+(\w*))?/i) {
> > + $ok++;
> > + $ref = $1;
> > + $email = $2 || ($replyto =~ /<(.*@.*)>/)[0];
>
> The e-mail address might not be "foo <bar@baz>"; it could be "bar@baz"
> or "bar@baz (foo)". I think there are Perl modules to handle details
> like this for you.
True. Same question as above, concerning modules.
> You've duplicated confirm here.
Will fix.
> As with my sendmessage() comment above, I think this is overengineered.
> It's fine (and, in fact, desirable) for subscribers to receive what the
> package maintainer sees, so it can just go in sendmailmessage().
Agreed.
> I sort of feel the need for a Debbugs::Subscriptions package here. I
> know modularization isn't exactly traditional in debbugs, but I wouldn't
> mind starting here ... However, I found Debbugs::Status a bit difficult
> to write because I needed to push all the file-finding functions out to
> Debbugs::Files or something, so I suppose we can do that later.
Would you want the module to export these functions and only separate
the source, or also do namespace sparation?
> I would prefer this function, and others that write files, to write to a
> temporary file and then rename it into place.
Sounds good, will do.
> What if somebody tries to subscribe to the same bug twice?
(assuming that $email is the same) he will have to re-confim it, but
since we read and write the whole thread, he won't be subscribed twice.
I will see if I can find a perl module that normalises the address well.
> It's probably better for locks to live in the 'lock' directory like all
> the others.
Agreed.
> > +sub subfile_write {
> > + my $file = shift;
> > + my $data = shift;
> > + open SUBFILE,">$file" || &quit("write $file: $!");
> > + while (my ($email,$hash) = each %$data) {
> > + print SUBFILE "$hash $email\n";
> > + }
> > + close SUBFILE;
> > +}
>
> You don't check for errors on print or close here (so you'll miss ENOSPC
> conditions).
Will do
>
> > +sub sendconfirmationmail {
> > + # Function to create a better mail asking for subscription
> Better than what? :)
I just wanted to put the text of the mail a bit separate. Currently, it
is a bit short...
Which brings my mind to: Was it ever considered to move all
e-mail-content away out of the source, e.g. in a module or using
something like TT?
> You don't seem to have arranged for subscribers to receive copies of
> control@ acknowledgements. I think this is a mistake: one good thing
> about bug subscriptions (although some might disagree when in a sneaky
> mood :)) are that we can arrange for submitters (by default) to be told
> about changes to things like severity and tags.
I will look into that.
> One thing you haven't addressed at all (perhaps deliberately) is the
> question of default subscriptions. It's not at all unreasonable to
> suggest that the submitter of a bug ought to be subscribed to it by
> default, and can either unsubscribe from it later or set some
> X-Debbugs-* header in the initial mail if this isn't desired. This way,
> we can start assuming that the submitter generally gets told about state
> changes to a bug, which I think would be an improvement.
I did not yet want to change any behaviour as it is now, but then - why
not. I will look into that, too.
> Since you wrote your original patch, we have extensible metadata in the
> form of .summary files. I think it would be reasonable to use those
> rather than having a separate file, which will also give you the benefit
> of the I/O functions already existing.
Yes, that makes sense.
> However, somebody will have to
> work out the field format: hashes as field values aren't currently
> supported, although there's no reason why they couldn't be made to work.
> Perhaps a one-space indent followed by
> escaped-key+whitespace+escaped-value or something like that would be OK.
What about using some serialisation module for this? I guess there are
quite a few around...
> Overall, this wasn't a bad effort, and I'm sorry it took us so long to
> get round to reviewing it. I think it needs a couple more iterations and
> a bit of discussion, but it should provide an excellent starting point.
Absolutely. I'll start right away getting used to the debbugs code base
again.
Thanks,
nomeata
--
Joachim "nomeata" Breitner
nomeata@debian.org | ICQ# 74513189 | GPG-Keyid: 4743206C
JID: joachimbreitner@amessage.de | http://people.debian.org/~nomeata
Attachment:
signature.asc
Description: Dies ist ein digital signierter Nachrichtenteil