-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Provide better documentation and help messages for x.py setup #77879
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
r? @jyn514 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! Lots of little nits, but the approach looks good.
Thanks for the review. I think I've addressed everything. I have only marked as "resolved" those comments where I did precisely what you suggested, so for the others you can decide for yourself if you agree with me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two other small nits, I agreed with the rest of your changes :)
Can you squash the commits a little? r=me after that. |
Thanks for working on this! |
I found another preexisting bug which I also fixed - see the final commit in the squashed series. |
@bors r+ |
📌 Commit 9dd00168a593bbb8c55e621a937552fd51eb5824 has been approved by |
☔ The latest upstream changes (presumably #77917) made this pull request unmergeable. Please resolve the merge conflicts. Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:
|
Rollup of 8 pull requests Successful merges: - rust-lang#77765 (Add LLVM flags to limit DWARF version to 2 on BSD) - rust-lang#77788 (BTreeMap: fix gdb provider on BTreeMap with ZST keys or values) - rust-lang#77795 (Codegen backend interface refactor) - rust-lang#77808 (Moved the main `impl` for FnCtxt to its own file.) - rust-lang#77817 (Switch rustdoc from `clean::Stability` to `rustc_attr::Stability`) - rust-lang#77829 (bootstrap: only use compiler-builtins-c if they exist) - rust-lang#77870 (Use intra-doc links for links to module-level docs) - rust-lang#77897 (Move `Strip` into a separate rustdoc pass) Failed merges: - rust-lang#77879 (Provide better documentation and help messages for x.py setup) - rust-lang#77902 (Include aarch64-pc-windows-msvc in the dist manifests) r? `@ghost`
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message
|
Put all()'s otuput in the order we want to print things in, and add a comment about why they are in this order. Provide purpose() and all_for_help(). Use these things everywhere. Move all the abbrev character ("a", "b", etc.) processing into interactive_path. No functional change. Signed-off-by: Ian Jackson <[email protected]>
Co-authored-by: Joshua Nelson <[email protected]>
We understand these profile names because we use .to_str(). Mention them in the question. Signed-off-by: Ian Jackson <[email protected]>
We need a fresh input buffer each time, or we reuse the previous data (since `read_line` appends). Signed-off-by: Ian Jackson <[email protected]>
EOF is not an error; it just causes read_line to produce "". Signed-off-by: Ian Jackson <[email protected]>
@bors r+ |
📌 Commit 636728e has been approved by |
⌛ Testing commit 636728e with merge 8d0cfd54d89abbac3a21d5121fda76fed10569df... |
💔 Test failed - checks-actions |
Spurious failure; it's the second time it's happened so I don't think there's much point in retrying. Opened https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/All.20apple.20builds.20are.20failing instead. |
Rollup of 9 pull requests Successful merges: - rust-lang#77570 (Allow ascii whitespace char for doc aliases ) - rust-lang#77739 (Remove unused code) - rust-lang#77753 (Check html comments) - rust-lang#77879 (Provide better documentation and help messages for x.py setup) - rust-lang#77902 (Include aarch64-pc-windows-msvc in the dist manifests) - rust-lang#77934 (Document -Z codegen-backend in the unstable book) - rust-lang#77936 (Remove needless alloc_slice) - rust-lang#77946 (Validate references to source scopes) - rust-lang#77951 (Update books) Failed merges: r? `@ghost`
Closes: #77861
I have split this up into tiny comments because I find it clearer this way. Feel free to squash it.