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

docs:Update rustc-dev-guide-book #135337

Closed

Conversation

QingyaoLin
Copy link

@QingyaoLin QingyaoLin commented Jan 10, 2025

Rename StringReader to Lexer and update links.

The StringReader struct link is invalid since pr:Merge TokenTreesReader into StringReader.

@rustbot
Copy link
Collaborator

rustbot commented Jan 10, 2025

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @lcnr (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added A-rustc-dev-guide Area: rustc-dev-guide S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 10, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 10, 2025

The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.

cc @BoxyUwU, @jieyouxu, @Kobzol

@QingyaoLin
Copy link
Author

This is the commit record for the StringReader changes I found in git log:

> git log -S 'struct StringReader' -- .\compiler\rustc_parse
commit 98777b4c490386ab7889718e811d28ce7423f0af
Author: Nicholas Nethercote <n.nethercote@gmail.com>
Date:   Thu Nov 14 16:55:27 2024 +1100

    Merge `TokenTreesReader` into `StringReader`.

@QingyaoLin
Copy link
Author

Since I'm working on Windows, I also can't build rustc-dev-guide, so I don't know why it doesn't detect that StringReader is a dead link. ( rustc-dev-guide uses mdbook-linkcheck2 to detect invalid links)

Does this mean that mdbook-linkcheck2 is a potential bug?

StringReader-link-invalid

@rustbot rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 11, 2025
@rustbot

This comment has been minimized.

@QingyaoLin
Copy link
Author

What happened here?

@Kobzol
Copy link
Contributor

Kobzol commented Jan 17, 2025

You pushed a merge commit; instead, you should use rebases, as per the guide in the comment above.

    Rename `StringReader` to `Lexer` and update links.

    The `StringReader` struct link is invalid since
    pr:Merge `TokenTreesReader` into `StringReader`.
@QingyaoLin QingyaoLin force-pushed the update-rustc-dev-guide-book branch from 51abac5 to 9b1f700 Compare January 17, 2025 15:46
@rustbot rustbot removed has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 17, 2025
@QingyaoLin
Copy link
Author

Fixed in rust-lang/rustc-dev-guide#2208

@QingyaoLin QingyaoLin closed this Jan 20, 2025
@Kobzol
Copy link
Contributor

Kobzol commented Jan 20, 2025

In general, if a PR only modifies rustc-dev-guide, it should be sent against that repository.

@QingyaoLin QingyaoLin deleted the update-rustc-dev-guide-book branch January 20, 2025 13:33
@QingyaoLin
Copy link
Author

QingyaoLin commented Jan 20, 2025

In general, if a PR only modifies rustc-dev-guide, it should be sent against that repository.一般来说,如果 PR 仅修改 rustc-dev-guide,则应针对该存储库发送它。

How should this be done? I don't quite understand how a PR for a large repository containing multiple sub-repositories should be targeted to the sub-repositories.

Can I do this directly in the rust-lang-rust repository, or should I clone it separately and modify the subrepo and pull the PR?
This seems to have a huge sense of disconnection.

@Kobzol
Copy link
Contributor

Kobzol commented Jan 20, 2025

If you only modify the code of a subtree (e.g. rustc-dev-guide), the PR should be made directly against the repository of the subtree repo (https://github.com/rust-lang/rustc-dev-guide). We regularly sync the code changes between rustc and the subtree repos.

We tried to document this better in #135770.

@QingyaoLin
Copy link
Author

Thanks,I understand.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 20, 2025
…uting, r=Kobzol

Update contributing docs for submodule/subtree changes

Noticed in rust-lang#135337 (comment).

r? `@Kobzol` (or anyone really)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 20, 2025
…uting, r=Kobzol

Update contributing docs for submodule/subtree changes

Noticed in rust-lang#135337 (comment).

r? ``@Kobzol`` (or anyone really)
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 20, 2025
Rollup merge of rust-lang#135770 - jieyouxu:subtree-submodule-contributing, r=Kobzol

Update contributing docs for submodule/subtree changes

Noticed in rust-lang#135337 (comment).

r? ``@Kobzol`` (or anyone really)
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Jan 22, 2025
…Kobzol

Update contributing docs for submodule/subtree changes

Noticed in rust-lang/rust#135337 (comment).

r? ``@Kobzol`` (or anyone really)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustc-dev-guide Area: rustc-dev-guide S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants