Post-commit feedback

When I previously was developing on LLVM, the natural place to give post-commit feedback on a patch was by replying to the llvm-commits email. While we have an mlir-commits mailing list, I don’t think anybody follows it (and e.g. phab reviews don’t go there).

What’s the community’s preferred way to give this kind of feedback?

In my most recent case, I just added a comment at the end of the Differential review that has already been committed, but it’s unclear to me how that percolates out and whether authors will see it. I’ve noticed that my Phab workflow recently is very much based on looking at the phab homepage with “Active Revisions” listing what needs attention, and I’m not sure what I would see if somebody made such a comment on my own diff.

1 Like

Thanks for bringing this up. Since my workflow is Phab based, I also almost missed a few comments that were added after submitting. I noticed them by chance because I also get an email notification, but given the volume, I often ignored the latter. Having post-commit feedback somehow show up more clearly would improve our reviewing process.

Doesn’t Phabricator sends emails on comments?

There is also the ability to leave comments on GitHub commits directly (e.g. 10bd67c1bd), but I don’t know if every commit authors would get an email on this?

Phabricator does send emails on comments, even for the differentials that have landed. But it you have wide email rules, e.g. notify on all diffs in mlir like I do, these emails tend to pass unnoticed due to high volume. I actually had given and received post-commit feedback like that.

I’ve recently noticed that some diffs somehow disappear from the front page and then reappear again, so I stopped looking at that.

I implemented the following workflow:

  • a gmail tag “my-commits” where I put the threads started by my diffs and check them periodically;
  • another tag “submitted” that is auto-assigned based on the presence of “to reflect the committed changes” in the body, so I can tell the if something needs more attention;
  • a filter that marks phabricator messages from myself as read.

This way, I can see if there were reactions to the diffs I landed as a combination of “my-commits”, “submit” and is:unread. There are two issues with these:

  • there is often a huge delay between phabricator actions and emails;
  • automated emails from continuous build show up even later and I learned that it is very often broken for a wrong reason so I’m tempted to auto-read them as well.

If github could figure out the association between the email address in the commit and a github account, and if the author has not opted out of github notifications.

Thanks folks. Sounds like this is something that we’ll need to figure out over time.

I recently signed up for mlir-commits, which seems to have a proper reply-to field with the author’s email (presumably derived from the commit email). So old-school “reply all” on those emails seems like it will work as it did in the past. I’ll give that a shot for a while.

btw, is there a way we can get Phab emails directed at mlir-commits as well? (@mehdi_amini ?)

So far I’ve been posting post-commit messages in the Diff but I also think most of them were missed :).

In my case, I just have a simple extra email filter, “LLVM To Me”, which filters out LLVM emails where my address is in the ‘To’ or ‘CC’ box. For Phab emails, your address will only be in the ‘To’ or ‘CC’ box when you are the author, reviewer or subscriber of a Diff. This works nicely even if Phab comments are added post-commit.