Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

loudly tell people when they change Cargo.lock #107631

Merged
merged 1 commit into from
Feb 4, 2023

Conversation

BoxyUwU
Copy link
Member

@BoxyUwU BoxyUwU commented Feb 3, 2023

It keeps happening that people accidentally commit changes to Cargo.lock and then have to be told by a reviewer to undo this. I've also seen cases where PRs are merged that accidentally changed Cargo.lock during a rebase.. I figure that purposeful changes to Cargo.lock are likely rarer than these accidental ones?

@rustbot
Copy link
Collaborator

rustbot commented Feb 3, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 3, 2023
@albertlarsan68
Copy link
Member

The pre-push hook does now fail if the lock file needs to be updated.

@BoxyUwU
Copy link
Member Author

BoxyUwU commented Feb 3, 2023

I don't think thats sufficient unless "now" means literally now, for example #107603 was submitted in the last 24hrs and has this issue 😅

@albertlarsan68
Copy link
Member

albertlarsan68 commented Feb 3, 2023

Since #107169 was merged, but they need to install the pre-push hook.
The PR CI will also fail if it needs to be updated.
The problem is more that people update the lock file needlessly.

@compiler-errors
Copy link
Member

I think this is fine to land as a stop-gap -- it's not too heavyweight of a ping on a PR, and those who know what they're doing (e.g. actually touching Cargo.toml) will ignore it.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 3, 2023

📌 Commit b83078f has been approved by compiler-errors

It is now in the queue for this repository.

@compiler-errors

This comment was marked as duplicate.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 3, 2023
@bors

This comment was marked as duplicate.

@bors
Copy link
Contributor

bors commented Feb 3, 2023

📌 Commit b83078f has been approved by compiler-errors

It is now in the queue for this repository.

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 3, 2023
…mpiler-errors

Rollup of 8 pull requests

Successful merges:

 - rust-lang#106887 (Make const/fn return params more suggestable)
 - rust-lang#107519 (Add type alias for raw OS errors)
 - rust-lang#107551 ( Replace `ConstFnMutClosure` with const closures )
 - rust-lang#107595 (Retry opening proc-macro DLLs a few times on Windows.)
 - rust-lang#107615 (Replace nbsp in all rustdoc code blocks)
 - rust-lang#107621 (Intern external constraints in new solver)
 - rust-lang#107631 (loudly tell people when they change `Cargo.lock`)
 - rust-lang#107632 (Clarifying that .map() returns None if None.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ef520bd into rust-lang:master Feb 4, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants