Skip to content
This repository has been archived by the owner on Mar 14, 2023. It is now read-only.

Mention notifications for changes to subtrees #337

Closed
calebcartwright opened this issue May 24, 2021 · 8 comments
Closed

Mention notifications for changes to subtrees #337

calebcartwright opened this issue May 24, 2021 · 8 comments

Comments

@calebcartwright
Copy link
Member

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
Copy link
Contributor

ehuss commented May 24, 2021

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.

@calebcartwright
Copy link
Member Author

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
Copy link
Contributor

ehuss commented May 24, 2021

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.

@calebcartwright
Copy link
Member Author

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 😞)

@jyn514
Copy link
Member

jyn514 commented May 24, 2021

This is a duplicate of #223.

@jyn514
Copy link
Member

jyn514 commented Jan 4, 2022

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.

@Mark-Simulacrum
Copy link
Member

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
Copy link
Contributor

ehuss commented Jun 28, 2022

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

@calebcartwright calebcartwright closed this as not planned Won't fix, can't repro, duplicate, stale Jun 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants