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

Clarify when cargo detects changes #11092

Merged
merged 2 commits into from
Sep 15, 2022
Merged

Clarify when cargo detects changes #11092

merged 2 commits into from
Sep 15, 2022

Conversation

TheButlah
Copy link
Contributor

@TheButlah TheButlah commented Sep 14, 2022

What does this PR try to resolve?

Its not clear if "any file" means "any rust file" or "any file, including non rust files". I think the confusion stemmed from the focus on it belonging to the rust package. So it was unclear if any file in the directory counts as belonging to the package, or if the package only contains rust source files, and therefore a .proto or .fbs file in the same directory as the Cargo.toml would not trigger a rebuild.

I amended the wording to focus on directories rather than packages.

How should we test and review this PR?

Please let me know which change it should be, and I will update my PR accordingly. Also, please let me know if my edit is accurate, or if it is misleading!

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 14, 2022
@epage
Copy link
Contributor

epage commented Sep 15, 2022

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Sep 15, 2022

📌 Commit 53a1064 has been approved by epage

It is now in the queue for this repository.

@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 Sep 15, 2022
@bors
Copy link
Contributor

bors commented Sep 15, 2022

⌛ Testing commit 53a1064 with merge aaadab6...

@bors
Copy link
Contributor

bors commented Sep 15, 2022

☀️ Test successful - checks-actions
Approved by: epage
Pushing aaadab6 to master...

@bors bors merged commit aaadab6 into rust-lang:master Sep 15, 2022
@TheButlah
Copy link
Contributor Author

Just to confirm, did you review to confirm that my revision actually reflects cargo's behavior? @epage

@TheButlah TheButlah deleted the patch-1 branch September 16, 2022 21:51
@epage
Copy link
Contributor

epage commented Sep 17, 2022

Gah, I misunderstood your PR and though you were saying how it is. I've not dug into this code yet to verify. As we are right around the cut off for branching off beta, I'm going to immediately revert and we can resolve this outside.

As stated in our contributing documentation, we prefer stuff to start as issues before moving on to PRs.

bors added a commit that referenced this pull request Sep 18, 2022
Revert "Clarify when cargo detects changes"

Reverts #11092

We had not fully confirmed that this language change matches the new behavior.
weihanglo added a commit to weihanglo/rust that referenced this pull request Sep 18, 2022
8 commits in 082503982ea0fb7a8fd72210427d43a2e2128a63..73ba3f35e0205844418260722c11602113179c4a
2022-09-13 17:49:38 +0000 to 2022-09-18 06:38:16 +0000

- Revert "Clarify when cargo detects changes" (rust-lang/cargo#11107)
- Fix links to workspace inheritance headings in workspace docs (rust-lang/cargo#11103)
- docs(ref): Clarify workspace settings (rust-lang/cargo#11082)
- Update comment about ResolveVersion default version (rust-lang/cargo#11095)
- [master] Run `reach_max_unpack_size` test only on debug build (rust-lang/cargo#11091)
- Clarify when cargo detects changes (rust-lang/cargo#11092)
- [master] Fix for CVE-2022-36113 and CVE-2022-36114 (rust-lang/cargo#11089)
- Expose cargo add internals as edit API (rust-lang/cargo#11059)
@TheButlah
Copy link
Contributor Author

Would that be a bug report? I didn't see a link to discuss documentation other than the forum

@epage
Copy link
Contributor

epage commented Sep 19, 2022

It could be framed as a bug report against the documentation for not being clear enough.

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 19, 2022
Update cargo (CVE fixes included)

8 commits in 082503982ea0fb7a8fd72210427d43a2e2128a63..73ba3f35e0205844418260722c11602113179c4a
2022-09-13 17:49:38 +0000 to 2022-09-18 06:38:16 +0000

- Revert "Clarify when cargo detects changes" (rust-lang/cargo#11107)
- Fix links to workspace inheritance headings in workspace docs (rust-lang/cargo#11103)
- docs(ref): Clarify workspace settings (rust-lang/cargo#11082)
- Update comment about ResolveVersion default version (rust-lang/cargo#11095)
- [master] Run `reach_max_unpack_size` test only on debug build (rust-lang/cargo#11091)
- Clarify when cargo detects changes (rust-lang/cargo#11092)
- [master] Fix for GHSA-rfj2-q3h3-hm5j and GHSA-2hvr-h6gw-qrxp (rust-lang/cargo#11089)
- Expose cargo add internals as edit API (rust-lang/cargo#11059)
@ehuss ehuss added this to the 1.65.0 milestone Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants