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.

Sign up for GitHub

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

Jump to bottom

fix some typos #94603

Closed
wants to merge 0 commits into from
Closed

fix some typos #94603

wants to merge 0 commits into from

Conversation

cuishuang
Copy link
Contributor

No description provided.

@rust-highfive
Copy link
Collaborator

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 4, 2022
@rust-highfive
Copy link
Collaborator

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 4, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Mar 4, 2022

Please rebase so there are no merge commits.

Are you finding these with a tool that we could regularly run? Or are there lots of false alarms?

@estebank estebank added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Mar 5, 2022

I think your rebase went wrong. Try rebasing over the master branch of this repo, that should clean it up

@bors
Copy link
Contributor

bors commented Mar 6, 2022

☔ The latest upstream changes (presumably #94579) made this pull request unmergeable. Please resolve the merge conflicts.

@cuishuang
Copy link
Contributor Author

☔ The latest upstream changes (presumably #94579) made this pull request unmergeable. Please resolve the merge conflicts.

Done.
Do I need to merge multiple commits into one? （use merge squash）

@oli-obk
Copy link
Contributor

oli-obk commented Mar 7, 2022

There are still merge commits left, you need to rebase and squash them all, yes.

Repeating this this question in case it got lost:

Are you finding these with a tool that we could regularly run? Or are there lots of false alarms?

@estebank
Copy link
Contributor

estebank commented Mar 7, 2022

Do I need to merge multiple commits into one? （use merge squash）

It's not always needed (when the work of each commit is self contained and the chain of commits has a narrative), but I also find that rebasing on top of master in complex PRs becomes much easier with squashed commits. Then you only have to deal with a single round of merge conflict resolution.

I would also like to highlight oli's question around how you found these. It'd be great if we could somehow mechanize part of that work so it doesn't have to be done manually (if the false positive rate can be kept at 0).

@cuishuang cuishuang force-pushed the master branch 2 times, most recently from 44fdef7 to da4c8f8 Compare March 9, 2022 13:13
@camelid
Copy link
Member

camelid commented Mar 11, 2022

The rebase still doesn't seem to be working. Can you try following the instructions here?

@bors
Copy link
Contributor

bors commented Mar 11, 2022

☔ The latest upstream changes (presumably #94824) made this pull request unmergeable. Please resolve the merge conflicts.

@Dylan-DPC Dylan-DPC closed this Mar 13, 2022
@Dylan-DPC Dylan-DPC mentioned this pull request Mar 15, 2022
Merged
@cuishuang
Copy link
Contributor Author

There are still merge commits left, you need to rebase and squash them all, yes.

Repeating this this question in case it got lost:

Are you finding these with a tool that we could regularly run? Or are there lots of false alarms?

Sorry, there have been a lot of things these days, and some work has been delayed

@cuishuang
Copy link
Contributor Author

Do I need to merge multiple commits into one? （use merge squash）

It's not always needed (when the work of each commit is self contained and the chain of commits has a narrative), but I also find that rebasing on top of master in complex PRs becomes much easier with squashed commits. Then you only have to deal with a single round of merge conflict resolution.

I would also like to highlight oli's question around how you found these. It'd be great if we could somehow mechanize part of that work so it doesn't have to be done manually (if the false positive rate can be kept at 0).

Yes, I previously maintained a map of fallible words for document correction. I guess there are similar tools on github and it is recommended to integrate it into CI

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 15, 2022
@matthiaskrgr
Rollup merge of rust-lang#94947 - Dylan-DPC:fix/typos, r=oli-obk
277802e 
fix typos

Rework of rust-lang#94603 which got closed as I was trying to unmerge and repush.  This is a subset of changes from the original pr as I sed'd whatever typos I remembered from the original PR

thanks to `@cuishuang` for the original PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@estebank estebank

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@cuishuang @rust-highfive @oli-obk @bors @estebank @camelid @rustbot @Dylan-DPC