When notify.reply is set and a changeset has a predecessor in the
repository, include In-Reply-To pointing to the message-id that would
have been generated for the oldest predecessor. This allows mail
threading like Phabricator for common cases like rebasing changes, but
will be optimal for cases like folding.
Details
- Reviewers
marmoute pulkit - Group Reviewers
hg-reviewers - Commits
- rHG947e6df4ff77: notify: optional mail threading based on obsmarker
Diff Detail
- Repository
- rHG Mercurial
- Branch
- default
- Lint
No Linters Available - Unit
No Unit Test Coverage
Event Timeline
The code looks good. Since the whole obsmarker thing is experimental, the config option here should be made experimental too.
Also, can you add tests when the config is set and --in-reply-to flag is used; and add info about that behavior in commit message?
@pulkit You still wanted to explain what you mean with --in-reply-to interaction here.
Thanks for the reminder. I looked into details and it seems that I misunderstood few things and mixed it with --in-reply-to flag to hg email :/
The feature looks interresting, but I had a couple of feedback.
hgext/notify.py | ||
---|---|---|
136 | maybe reply-to-predecessors would be clearer ? | |
137 | The phrasing used in the rest of the doc is If set | |
139 | From waht I understand, this feature will misbehave if notify.messageidseed is not set. I am I right, I believe we should replace "should" by "must" and abort if we detect the new option being used without the other one. | |
466–470 | nits: could be [] instead of list() | |
472 | The "min" does not garantee we are using the root predecessors here. It is just an attemps to get the first that resulted in an email, right? If so, you should document it so that the next soul looking at this code do not get mislead. You should also document that in case of fold, only one will be replied to. |
Looks good to me, thanks for the update. Sorry for the delay, it totally fell into the cracks.
hgext/notify.py | ||
---|---|---|
472 | I was thinking of an inline comment in the code. Having this in the doc is actually good idea thanks. Maybe follow up with an inline command too ? |
maybe reply-to-predecessors would be clearer ?