Skip to content

Typos batch: lib and include folders #75020

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

Closed
wants to merge 2 commits into from
Closed

Typos batch: lib and include folders #75020

wants to merge 2 commits into from

Conversation

Sajjon
Copy link
Contributor

@Sajjon Sajjon commented Jul 7, 2024

Fix typos to lib and include folders, as per suggestion by @compnerd

If valuable, I did split the work up initially, in my fork, in two PRs merged into this source branch, both of which, in turn was aggregated from several separate PRs.

(most likely we wanna squash those 22 commits into a single one...)

Copy link
Contributor

@al45tair al45tair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of these changes aren't right and should be fixed.

@al45tair
Copy link
Contributor

al45tair commented Jul 8, 2024

@swift-ci Please test

@al45tair
Copy link
Contributor

al45tair commented Jul 8, 2024

@swift-ci Please test

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Sajjon 🙏🏽 I reviewed the following directories:

  • include/swift/IDE
  • include/swift//IDETool
  • include/swift/Parse
  • include/swift/Refactoring
  • lib/ASTGen
  • lib/IDE
  • lib/IDETool
  • lib/Macros
  • lib/Parse
  • lib/Refactoring

@Sajjon
Copy link
Contributor Author

Sajjon commented Aug 8, 2024

@al45tair @ahoppen I've rebased on main, fixed conflicts and squashed into a single commit.

So should be re-review / merge ready

@ahoppen
Copy link
Member

ahoppen commented Aug 8, 2024

@swift-ci Please smoke test

@@ -3380,7 +3380,7 @@ void SILFunction::print(SILPrintContext &PrintCtx) const {
case IsReabstractionThunk: OS << "[reabstraction_thunk] "; break;
}
if (isDynamicallyReplaceable()) {
OS << "[dynamically_replacable] ";
OS << "[dynamically_replaceable] ";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AnthonyLatsis should revert, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can change it, but in this case you have to change all occurrences in all test files, too. If you want to do this, I recommend to put those changes in a separate commit

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would extract that change into a separate PR, this one is quite large already. If you choose to revert, please make sure to audit all modified occurrences.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm leaving it as is.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then please put everything related to dynamically_replaceable in a separate commit, and make sure to modify all occurrences across the repository. Otherwise the tests will not pass, and we don’t want comments and documentation that mention this compiler attribute to be out-of-date.

Copy link
Contributor Author

@Sajjon Sajjon Aug 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AnthonyLatsis I've changed all occurrence to -> dynamically_replacable in separate commit: 08ba5a5

@vivoxfold3
Copy link

⚠️⚠️⚠️Caution⚠️⚠️⚠️

This user is attempting to execute the first step of a supply chain attack, attempting to break free from the 'first-time contributor' status. The user opened a similar PR in all other repositories on the same day. He also stole pictures of other users to make them look like real people.

There are still hundreds of submissions like this, and they can be fixed at once. They will be submitted in multiple batches, and this is one part of them

@Sajjon

This comment has been minimized.

@adrian-prantl
Copy link
Contributor

I feel uncomfortable with the size and contents of this patch. There are so many changes that none of the reviewers are going to read the entire patch, which would make it really easy for something slip through review here. Since none of these changes depend on each other, would you mind splitting this up into many smaller PRs that either all fix the same typo, or fix only typos in one directory so there's a smaller set of reviewers for each PR?

@Sajjon Sajjon closed this by deleting the head repository Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants