Page MenuHomeFreeBSD

Make it possible not to send MFH emails
Needs ReviewPublic

Authored by adamw on Jun 19 2019, 12:25 PM.

Details

Reviewers
delphij
Group Reviewers
ports secteam
portmgr
Summary

This treats

MFH:     2019Q2 (any text)

as a signal not to send the MFH request email.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

adamw created this revision.Jun 19 2019, 12:25 PM
adamw updated this revision to Diff 58792.Jun 19 2019, 12:26 PM

Oops, there was a typo there.

mat added a comment.Jun 19 2019, 3:48 PM

I don't know about this, I think it is better to send an email that is ignored than silently doing things.

delphij accepted this revision.Jun 19 2019, 3:49 PM
This revision is now accepted and ready to land.Jun 19 2019, 3:49 PM
In D20698#447294, @mat wrote:

I don't know about this, I think it is better to send an email that is ignored than silently doing things.

I think ultimately what happens on quarterly branch is still being covered by commit mail? This only affects MFH approval email, which seems to be redundant if it's a hat commit and the intention was not to ask an approval in the first place.

mat added a comment.Jun 19 2019, 3:57 PM
In D20698#447294, @mat wrote:

I don't know about this, I think it is better to send an email that is ignored than silently doing things.

I think ultimately what happens on quarterly branch is still being covered by commit mail? This only affects MFH approval email, which seems to be redundant if it's a hat commit and the intention was not to ask an approval in the first place.

Then if the MFH line contains anything else than a branch name, say:

MFH: 2019Q2 (security update blanket approval)

There is no need to send an email because it will already have gotten committed.

Maybe it should echo the fact that no MFH email is being sent. (So need two tests.)

In D20698#447300, @mat wrote:

Then if the MFH line contains anything else than a branch name, say:

MFH: 2019Q2 (security update blanket approval)

There is no need to send an email because it will already have gotten committed.

That's what the above patch does.

Maybe it should echo the fact that no MFH email is being sent. (So need two tests.)

That's a great idea. Instant feedback would help alleviate the behaving-silently thing.

mat added a comment.Jun 20 2019, 10:44 AM
In D20698#447300, @mat wrote:

Then if the MFH line contains anything else than a branch name, say:

MFH: 2019Q2 (security update blanket approval)

There is no need to send an email because it will already have gotten committed.

That's what the above patch does.

That's what it does only when you write "with hat". What I mean is that if you add a comment on the MFH line, whatever the comment is, an email should not be sent.

adamw edited the summary of this revision. (Show Details)Jun 20 2019, 12:27 PM
adamw updated this revision to Diff 58830.

I see what you mean. Here it takes anything between parentheses as a sign to exit.

This revision now requires review to proceed.Jun 20 2019, 12:27 PM
mat added inline comments.Jun 20 2019, 2:31 PM
hooks/scripts/mfh.sh
9

I'm not sure you need to change this line, it does not change anything wrt what gets in MFH_LINE in the end.

10–12

Splitting it in two to get a message out:

if [ "$MFH_LINE" = "" ]; then
        exit 0
fi
if echo "$MFH_LINE" | grep -q '(.*)'; then
        echo "Comment detected on the MFH line," 1>&2
        echo "not sending the MFH email." 1>&2
        exit 0
fi