-
Notifications
You must be signed in to change notification settings - Fork 68
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
Fix all remaining spelling errors throughout the codebase #213
base: main
Are you sure you want to change the base?
Conversation
...to the best of my knowledge. I used cspell (`brew install cspell`) by running `cspell lint -u --words-only --no-progress "**/*" | sort > words.txt`, and then manually scanned through the list of around 2000 derived errors, skipping past technical terms or compound words that were valid but not in cspell's dictionaries. This also standardized the spelling of a handful of words to US English, and tweaked the grammar of the surrounding sentence structure in a few cases where obscure constructed or slang words were used. The majority of changes are in comments only, and most others in local variable names or test function names.
@swift-ci test |
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.
Nicely done! This tool seems excellent, and I believe we could make it mandatory for use on all PRs.
I’ve reviewed all the changes manually and found only one minor change that may need to be reconsidered, while the rest appear to be more suited for discussion.
Please let me know your thoughts on automating the use of this tool.
@@ -149,7 +149,7 @@ extension BuildRequestContext { | |||
|
|||
let currentPlatformFilter = PlatformFilter(settings.globalScope) | |||
|
|||
// FIXME: It is a bit unfortunate that we need to compute all this for the `uniquingSuffix` behaviour. |
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.
Behaviour
is the correct spelling in British English. We may need to introduce a simple guideline to standardize either British or US spelling.
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.
I see you already mentioned it in the PR description. My point is to establish a clear code of conduct that explicitly requires running a tool like this across the code for any change, or simply making it mandatory for every change moving forward.
@@ -66,7 +66,7 @@ final class ShellScriptTaskProducer: PhasedTaskProducer, TaskProducer, ShellBase | |||
// This is to provide a fallback to prevent any unexpected side-effects this close to shipping... | |||
guard scope.evaluate(BuiltinMacros.ENABLE_ADDITIONAL_CODESIGN_INPUT_TRACKING) else { return } | |||
|
|||
// Due to the free-form nature of how script outputs can be formed, it can be problematic to blanketly add these as doing so can trigger cycles from downstream targets. | |||
// Due to the free-form nature of how script outputs can be formed, it can be problematic to blanket add these as doing so can trigger cycles from downstream targets. |
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.
I think it should be blindly
instead.
overrides: ["ENABLE_USER_SCRIPT_SANDBOXING": enableSandboxingInTest, | ||
"A_DOT_TXT_PREFIX": "UPDATED_A_PREFIX", | ||
"B_DOT_TXT_PREFIX": "B_PREFIX", | ||
]) |
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.
Are these formatting changes necessary in this PR? Just asking
@@ -3003,7 +3003,7 @@ import SWBMacro | |||
"ENABLE_XOJIT_PREVIEWS": "YES", | |||
"CODE_SIGN_IDENTITY": "An Engineer", | |||
|
|||
"ENABLE_HARDED_RUNTIME_EXPECTED": "YES", |
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.
Is this an indication that the test is incorrect, since it didn’t fail with an invalid flag?
...to the best of my knowledge.
I used cspell (
brew install cspell
) by runningcspell lint -u --words-only --no-progress "**/*" | sort > words.txt
, and then manually scanned through the list of around 2000 derived errors, skipping past technical terms or compound words that were valid but not in cspell's dictionaries.This also standardized the spelling of a handful of words to US English, and tweaked the grammar of the surrounding sentence structure in a few cases where obscure constructed or slang words were used.
The majority of changes are in comments only, and most others in local variable names or test function names.