Fix misspellings in code files #1420
Conversation
aleksdmladenovic
temporarily deployed
to
production
GitHub Actions
Inactive
aleksdmladenovic
temporarily deployed
to
production
GitHub Actions
Inactive
aleksdmladenovic
temporarily deployed
to
production
GitHub Actions
Inactive
|
Hope you to check this. |
aleksdmladenovic
temporarily deployed
to
production
GitHub Actions
Inactive
There was a problem hiding this comment.
Thanks!
Generally lgtm, although I'd probably prefer the previous spellings of Cachable,Uncachable and Unreachable as they're more concise and still valid spellings: https://en.wiktionary.org/wiki/cachable, https://en.wiktionary.org/wiki/unreachable
Reviewed 29 of 29 files at r1, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained, and all files reviewed, and pending CI: Remote / large-ubuntu-22.04, and 1 discussions need to be resolved
nativelink-worker/tests/utils/mock_running_actions_manager.rs line 106 at r1 (raw file):
RunningActionManagerCalls::CacheActionResult(req) => *req, RunningActionManagerCalls::CreateAndAddAction(_) => { panic!("Got incorrect call waiting for cache_action_result")
nit: is this necessary?
There was a problem hiding this comment.
Well, it seemed like that word usage percentage of cacheable is bigger than cachable.
But it depends on our specific purposes.
So do you want to just go with cachable as before? cc: @aaronmondal
Reviewable status: 1 of 2 LGTMs obtained, and all files reviewed, and pending CI: Remote / large-ubuntu-22.04, and 1 discussions need to be resolved
nativelink-worker/tests/utils/mock_running_actions_manager.rs line 106 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: is this necessary?
It was from the running result of
`bazel run \
--@rules_rust//:rustfmt.toml=//:.rustfmt.toml \
@rules_rust//:rustfmt`
command on my side.
aleksdmladenovic
force-pushed
the
fix-code-misspellings
branch
from
b0e6b0d to
1217b49
Compare
aleksdmladenovic
temporarily deployed
to
production
GitHub Actions
Inactive
aleksdmladenovic
temporarily deployed
to
production
GitHub Actions
Inactive
aleksdmladenovic
temporarily deployed
to
production
GitHub Actions
Inactive
aleksdmladenovic
temporarily deployed
to
production
GitHub Actions
Inactive
There was a problem hiding this comment.
Addressed your changes. @aaronmondal
Hope you to check thsi and merge PR.
Thank you!
cc: @MarcusSorealheis, @allada
Reviewable status: 1 of 2 LGTMs obtained, and 15 of 29 files reviewed, and pending CI: Analyze (python), Bazel Dev / macos-13, Bazel Dev / macos-14, Bazel Dev / ubuntu-24.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Coverage, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Local / ubuntu-22.04, NativeLink.com Cloud / Remote Cache / macos-14, NativeLink.com Cloud / Remote Cache / ubuntu-24.04, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Remote / large-ubuntu-22.04, Web Platform Deployment / macos-14, Web Platform Deployment / ubuntu-24.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (22.04), integration-tests (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, and 1 discussions need to be resolved
There was a problem hiding this comment.
You can rebase the PR, then the workflows shouldn't break.
Reviewable status: 1 of 2 LGTMs obtained, and 15 of 29 files reviewed, and pending CI: Remote / large-ubuntu-22.04, and 1 discussions need to be resolved
Currently, we are doing an strict misspelling check on document files but not on code files. Fixed all misspellings according to the intensive misspelling check on all code files.
aleksdmladenovic
force-pushed
the
fix-code-misspellings
branch
from
1217b49 to
f35e048
Compare
aleksdmladenovic
temporarily deployed
to
production
GitHub Actions
Inactive
aleksdmladenovic
temporarily deployed
to
production
GitHub Actions
Inactive
aleksdmladenovic
temporarily deployed
to
production
GitHub Actions
Inactive
aleksdmladenovic
temporarily deployed
to
production
GitHub Actions
Inactive
There was a problem hiding this comment.
Rebased PR. cc: @SchahinRohani, @MarcusSorealheis
Reviewable status: 1 of 2 LGTMs obtained, and 13 of 29 files reviewed, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / macos-13, Bazel Dev / macos-14, Bazel Dev / ubuntu-24.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Coverage, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Local / ubuntu-22.04, NativeLink.com Cloud / Remote Cache / macos-14, NativeLink.com Cloud / Remote Cache / ubuntu-24.04, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Remote / large-ubuntu-22.04, Web Platform Deployment / macos-14, Web Platform Deployment / ubuntu-24.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (22.04), integration-tests (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, and 1 discussions need to be resolved
There was a problem hiding this comment.
Reviewed 1 of 5 files at r3.
Reviewable status: 1 of 2 LGTMs obtained, and 14 of 29 files reviewed, and 1 discussions need to be resolved
There was a problem hiding this comment.
Reviewable status: 2 of 2 LGTMs obtained, and 14 of 29 files reviewed, and 1 discussions need to be resolved
There was a problem hiding this comment.
Reviewed 1 of 14 files at r2, 5 of 5 files at r3, all commit messages.
Reviewable status: 2 of 2 LGTMs obtained, and 19 of 29 files reviewed, and 1 discussions need to be resolved
There was a problem hiding this comment.
Reviewed 10 of 14 files at r2.
Reviewable status:complete! 2 of 2 LGTMs obtained, and all files reviewed
Description
Currently, we are doing an strict misspelling check on document files but not on code files. According to intensive misspelling check on all code files, catched 92 occurrences in 27 files and fixed.
Fixes #1419
Type of change
Checklist
bazel test //...passes locallygit amendsee some docsThis change is