Make clippy a subrepo #70650
Closed
Make clippy a subrepo #70650
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…src/tools/clippy subrepo: subdir: "src/tools/clippy" merged: "c211cea3e99" upstream: origin: "https://github.com/rust-lang/rust-clippy.git" branch: "auto" commit: "c211cea3e99" git-subrepo: version: "0.4.1" origin: "???" commit: "???"
rust-highfive
added
the
S-waiting-on-review
RalfJung
reviewed
Apr 1, 2020
RalfJung
reviewed
Apr 1, 2020
RalfJung
reviewed
Apr 1, 2020
RalfJung
reviewed
Apr 1, 2020
RalfJung
reviewed
Apr 1, 2020
RalfJung
reviewed
Apr 1, 2020
RalfJung
reviewed
Apr 1, 2020
RalfJung
reviewed
Apr 1, 2020
CONTRIBUTING.md
Outdated
| happened to the copy in this repo and creates commits on the remote repo that match the local | ||
| changes (so ever local commit that touched the subrepo causes a commit on the remote repo). Again, | ||
| Even this `push` operation creates a commit in this repo that you need to get merged without | ||
| rebasing. This is very important in order to make future synchronizations work. |
There was a problem hiding this comment.
So the idea is, I will git subrepo push, and then make a rustc PR (likely self-r+ p=1) and a tool PR? And then what, I wait until the rustc PR lands before landing the tool PR, or vice versa? Either way there is a chance of conflicts where one PR lands and the other one does not, and it is unclear how bad that is. We should give some general guidance to at least have a consistent process.
There was a problem hiding this comment.
We have no experience here. Not sure how to build guidance without really knowing how it will behave in practice.
There was a problem hiding this comment.
Come up with a best guess, adjust it later if it doesn't work? ;)
RalfJung
reviewed
Apr 1, 2020
CONTRIBUTING.md
Outdated
| monorepo, so no one but you (or others that synchronize subrepos) needs to have `git subrepo` | ||
| installed. | ||
|
|
||
| #### Synchronizing a subrepo |
There was a problem hiding this comment.
This, too, is something that contributors usually shouldn't have to do, instead that would generally be tool maintainer's job?
Is there a way for us maintainers to be notified when our tools change in rustc so that we know we need to sync? (Longer-term, having a bot to also do the syncing would be even better of course. But some kind of notification is required, I think, to avoid silent divergence.)
There was a problem hiding this comment.
Contributors shouldn't care about this entire section. Where else do I put it?
We could probably extend the toolstate bot to check whether git diff src/tools/clippy has any output, and in that case ping the team
RalfJung
reviewed
Apr 1, 2020
| missing_tool("cargo clippy", builder.build.config.missing_tools); | ||
| None | ||
| })?; | ||
| .expect("clippy expected to build - essential tool"); |
There was a problem hiding this comment.
The config.toml.example still lists clippy in the tool list and as an example of an "extended tool". Is that outdated now?
There was a problem hiding this comment.
kinda, you can still turn it off to not include it in the dist
|
Checklist:
Reassigning to r? @Mark-Simulacrum since they are more familiar with this infra and cc @pietroalbini |
|
Closing because there is more discussion to be had on the issue and this PR will regress soon anyway |
|
Just keep that contribution guide somewhere please, now that we edited it a bit. :) |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
r? @Centril
Please keep all general subrepo discussion in #70651 to keep everyone's mentions clean from any kind of bors activity that we may have here.