Code review comment for ~michal-maloszewski99/ubuntu/+source/amavisd-new:merge-1.2.13.0-3_mantic

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Well, 07-26 fixed what I wanted - which was mostly how to overcome the FTBFS in a better way.
You have taken over my WIP suggestion, which I assume that means worked well in your tests as you said it it is ready for re-review.

Then on 08-02 Bryce was so kind to beat me in giving this a second look - thanks.

I ack to all of Bryce suggestions, but I also see your reason for hesitation - let me explain.

First time you presented the delta re-split in a different way and re-ordered its appearance in changelog - that made it hard to compare and match.
Then after my work with you we re-ordered it so that old=new (in regard to order) which helped a lot - but we didn't touch the text of the messages as we were both quite exhausted at that time already from all the shuffling.
Now Bryce asks you to change the texts and I can understand thinking "uh, wasn't this what I tried before" :-)
But he is not asking you to change order again, everything stays in place and just gets a nice improvement in wording.
I'm +1 with that and encourage you to do that so that I can final-review and final-approve it then.

One thing Bryce asked was to not say "after discussion with the server team" which I agree.
But then, you just carried forward this - and the discussion is from 2012 let me help a bit :-)
When you go back in the past enough in the CL you see:
  [6aa7cfb] Recommend altermime and ripole (Closes: #665469)
And as a consequence of that
  * debian/control: drop altermime and ripole to Suggests after discussions
    with the server team. (LP: 992879)

So there you at least have a bug ref.
And since then no one complained about those being missing, so the decision seems to have been right :-)

A good message might therefore
1. would state that as the reason
2. still reference the old bug (which was lost in CL a few years later)
So a good message could be:
 * d/control: drop altermime and ripole to Suggests as they are optional and universe dependencies (LP: 992879)

But no problem would be good without being a never ending rabbit whole - right?
In looking at this I've seen ripole doesn't exist since >Bionic.
Which means it is wrong in Debian (and known https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=971896)
So you can change this to:
1. just move altermime, keep ripole a recommends as it is a no-op due to not existing
2. the commit and changelog text then could be
 * d/control: drop altermime to Suggests as it is an optional universe dependency (LP: 992879)

Oh and a last comment - as this is the repo with so many similar branches.
You could maybe delete the wrong old branches in your LP repo to make this easier?

Once this is all ready, ping me for a final check and sponsoring

« Back to merge proposal