rust-lang/highfive

Mention notifications for changes to subtrees

calebcartwright opened this issue ยท 8 comments

Admittedly not sure at this point if this is just questions or a bug report, and if it's the latter then it's likely a duplicate of #223

Encountered Behavior
There's a mention configuration entry for rust-lang/rust to notify me when there's changes to src/tools/rustfmt, which was working when rustfmt was a submodule (at least there were a few times the notification occurred).

However, since the conversion of rustfmt from a submodule to a subtree there's been a few PRs in rust-lang/rust which modified the rustfmt source in the subtree, but the bot didn't trigger any notification (e.g. rust-lang/rust#84571)

"src/tools/rustfmt": {
"message": "Some changes occurred in src/tools/rustfmt.",
"reviewers": ["@calebcartwright"]
},

Questions

  • Is there any issues/differences for notifications on submodules vs subtrees?
  • Are the mention notifications for changes to configured files/directories only applicable upon PR creation or are they supposed to also be applied on subsequent changes to an existing PR?
ehuss commented

mentions are only processed when a PR is opened. I'm guessing that PR did not touch src/tools/rustfmt when it was first opened. There isn't much of a difference for submodules, it just looks at git diff for files that start with the given path.

Gotcha, thanks @ehuss! Do you know if there's any configurations here and/or any other bots which can be used to provide notifications for such changes, regardless of whether they're on the initial PR?

ehuss commented

I'm not aware of any used in rust-lang. highfive would need to be enhanced to look at every push (and avoid duplicate mentions). However, I'm uncertain if infra would prefer more extensive changes like that be made in triagebot instead.

The concern I have is that the formatting stability guarantee of rustfmt also applies to nightly builds, but that's not really something we can manage/guarantee if folks continue to have the ability to update the rustfmt source in-tree without the rustfmt team having any awareness of such changes.

Will try to figure out how to work around this in the short term but longer term we'd need some kind of informed/consulted process in place for the subtree to continue to be viable, otherwise I think we'd need to consider going back to a submodule (as unpleasant and unfortunate as that would be ๐Ÿ˜ž)

This is a duplicate of #223.

This is mostly done on the triagebot side since rust-lang/triagebot#1321, someone just needs to hook up the file notifications to a config setting somewhere.

Ah, well, it would need to gain the ability to ping folks (it currently only sets labels), but yes, that's right. I'd be happy to review a PR -- at a guess, it should be pretty simply to plumb that through, maybe 1 hour of work at most. Feel free to poke me on Zulip or elsewhere if someone is interested in doing it.

ehuss commented

This should now essentially be fixed in triagebot, so I think this could probably be closed.