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

Add codespell: workflow, #1888

Merged
merged 9 commits into from
Nov 28, 2023

Conversation

yarikoptic
Copy link
Contributor

Submission Checklist

Please take the following actions before submitting this pull request.

  • Fork your PR branch from the Toolkit "develop" branch (not main)
  • Test all changes with pre-commit in a local branch #. If running with -a codespell triggers since then pre-commit passes all file names and thus excludes in .codespellrc do not apply. Should I do anything about that?
  • Confirm that "make tests" passes all tests
  • Follow Cloud HPC Toolkit Contribution guidelines #

@nick-stroud nick-stroud self-assigned this Nov 21, 2023
@nick-stroud nick-stroud self-requested a review November 21, 2023 21:30
Copy link
Collaborator

@nick-stroud nick-stroud 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 for the submission and apologies that this has sit for a while.

Please make the requested changes and then un-assign yourself from the PR.

pkg/config/expression.go Outdated Show resolved Hide resolved
.github/workflows/codespell.yml Outdated Show resolved Hide resolved
@nick-stroud
Copy link
Collaborator

/gcbrun

@nick-stroud nick-stroud added the release-improvements Added to release notes under the "Improvements" heading. label Nov 22, 2023
@yarikoptic yarikoptic removed their assignment Nov 24, 2023
@yarikoptic
Copy link
Contributor Author

Please make the requested changes and then un-assign yourself from the PR.

done! also rebased and reran the codespell -w last commit -- there were new typos (or just new detected with newer codespell I used)

Here is the interdiff between those two showing new fixups
❯ interdiff -U1 <(git show gh-yarikoptic/enh-codespell) <(git show enh-codespell)
diff -U1 b/community/front-end/ofe/website/ghpcfe/views/credentials.py b/community/front-end/ofe/website/ghpcfe/views/credentials.py
--- b/community/front-end/ofe/website/ghpcfe/views/credentials.py
+++ b/community/front-end/ofe/website/ghpcfe/views/credentials.py
@@ -130,3 +130,3 @@
 class CredentialViewSet(viewsets.ModelViewSet):
-    """Custom ModelViewSet for Crendential model"""
+    """Custom ModelViewSet for Credential model"""
 
diff -U1 b/community/modules/compute/gke-job-template/variables.tf b/community/modules/compute/gke-job-template/variables.tf
--- b/community/modules/compute/gke-job-template/variables.tf
+++ b/community/modules/compute/gke-job-template/variables.tf
@@ -41,3 +41,3 @@
 variable "node_pool_name" {
-  description = "A list of node pool names on which to run the job. Can be populated via `use` field."
+  description = "A list of node pool names on which to run the job. Can be populated via `use` feild."
   type        = list(string)
@@ -53,3 +53,3 @@
 variable "node_pool_name" {
-  description = "A list of node pool names on which to run the job. Can be populated via `use` feild."
+  description = "A list of node pool names on which to run the job. Can be populated via `use` field."
   type        = list(string)
diff -U1 b/community/modules/file-system/nfs-server/README.md b/community/modules/file-system/nfs-server/README.md
--- b/community/modules/file-system/nfs-server/README.md
+++ b/community/modules/file-system/nfs-server/README.md
@@ -120,3 +120,3 @@
 | <a name="input_auto_delete_disk"></a> [auto\_delete\_disk](#input\_auto\_delete\_disk) | Whether or not the nfs disk should be auto-deleted | `bool` | `false` | no |
-| <a name="input_deployment_name"></a> [deployment\_name](#input\_deployment\_name) | Name of the HPC deployment, used as name of the NFS instance if no name is specified. | `string` | n/a | yes |
+| <a name="input_deployment_name"></a> [deployment\_name](#input\_deployment\_name) | Name of the HPC deployment, used as name of the NFS instace if no name is specified. | `string` | n/a | yes |
 | <a name="input_disk_size"></a> [disk\_size](#input\_disk\_size) | Storage size gb | `number` | `"100"` | no |
@@ -125,3 +125,3 @@
 | <a name="input_auto_delete_disk"></a> [auto\_delete\_disk](#input\_auto\_delete\_disk) | Whether or not the nfs disk should be auto-deleted | `bool` | `false` | no |
-| <a name="input_deployment_name"></a> [deployment\_name](#input\_deployment\_name) | Name of the HPC deployment, used as name of the NFS instace if no name is specified. | `string` | n/a | yes |
+| <a name="input_deployment_name"></a> [deployment\_name](#input\_deployment\_name) | Name of the HPC deployment, used as name of the NFS instance if no name is specified. | `string` | n/a | yes |
 | <a name="input_disk_size"></a> [disk\_size](#input\_disk\_size) | Storage size gb | `number` | `"100"` | no |
--- b/community/modules/scripts/ramble-setup/main.tf
+++ a/community/modules/scripts/ramble-setup/main.tf
@@ -24,3 +24,3 @@
     if [ -f ${var.install_dir}/share/ramble/setup-env.sh ]; then
-          echo "** Ramble's python virtualenv (/usr/local/ramble-python) is activated. Call 'deactivate' to deactivate."
+          echo "** Ramble's python virtualenv (/usr/local/ramble-python) is actiavted. Call 'deactivate' to deactivate."
           . /usr/local/ramble-python/bin/activate
diff -U1 b/modules/README.md b/modules/README.md
--- b/modules/README.md
+++ b/modules/README.md
@@ -473,3 +473,3 @@
 `terraform output`. This can useful for displaying the IP of a login node or
-printing instructions on how to use a module, as we have in the
+priting instructions on how to use a module, as we have in the
 [monitoring dashboard module](monitoring/dashboard/README.md#Outputs).
@@ -489,3 +489,3 @@
 `terraform output`. This can useful for displaying the IP of a login node or
-priting instructions on how to use a module, as we have in the
+printing instructions on how to use a module, as we have in the
 [monitoring dashboard module](monitoring/dashboard/README.md#Outputs).
only in patch2:
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -197,3 +197,3 @@
 	}
-	// Code bellow assumes that `args0` contains path to file, not a
+	// Code below assumes that `args0` contains path to file, not a
 	// executable name from PATH.
only in patch2:
--- a/tools/maintenance/maintenance.py
+++ b/tools/maintenance/maintenance.py
@@ -168,3 +168,3 @@
     parser.add_argument("-m", "--print_periodic_vms", action="store_true",
-                        help="Disply nodes that have periodic" \
+                        help="Display nodes that have periodic" \
                              " maintenance setup")

@nick-stroud nick-stroud self-assigned this Nov 27, 2023
@nick-stroud
Copy link
Collaborator

/gcbrun

@rohitramu
Copy link
Collaborator

/gcbrun

@nick-stroud nick-stroud removed their assignment Nov 27, 2023
@rohitramu
Copy link
Collaborator

@yarikoptic It looks like the codespell pre-commit is failing right now in the PR-validation check:

codespell................................................................Failed
- hook id: codespell
- exit code: 65

If you run the pre-commit locally, do you get the same error?

@nick-stroud
Copy link
Collaborator

It looks like the pre-commit does not respect the .codespellrc file.

For example here is a sample of the errors:

Step #2 - "pre-commit-run": community/front-end/ofe/requirements.txt:4: astroid ==> asteroid
Step #2 - "pre-commit-run": community/front-end/ofe/website/ghpcfe/static/js/bootstrap.bundle.js:2947: trough ==> through
Step #2 - "pre-commit-run": community/front-end/ofe/website/ghpcfe/static/js/bootstrap.bundle.js:3420: othwerise ==> otherwise

These are in .js and requirements.txt files.

I was able to reproduce this locally by running pre-commit run codespell -a.

@yarikoptic, reassigning to you. See if you can get these skipped files observed by the pre-commit. Un-assign when you are ready for re-review. Thanks!

@nick-stroud nick-stroud assigned yarikoptic and unassigned rohitramu Nov 27, 2023
=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w -i 3 -C 2 ./community/front-end/ofe/script/service_account.sh ./community/front-end/ofe/website/ghpcfe/models.py ./community/front-end/ofe/website/ghpcfe/models.py ./community/modules/scripts/htcondor-install/files/autoscaler.py ./community/modules/scripts/ramble-setup/README.md ./docs/videos/healthcare-and-life-sciences/README.md ./examples/README.md ./tools/validate_configs/test_configs/README.md",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
@yarikoptic
Copy link
Contributor Author

sorry about that.

@rohitramu
Copy link
Collaborator

/gcbrun

@rohitramu rohitramu assigned rohitramu and unassigned yarikoptic Nov 28, 2023
@rohitramu rohitramu merged commit 4d9a39a into GoogleCloudPlatform:develop Nov 28, 2023
6 of 35 checks passed
ek-nag pushed a commit to ek-nag/hpc-toolkit that referenced this pull request Nov 28, 2023
* Add rudimentary codespell config

* Add pre-commit definition for codespell

* ot -> it typo fix

* Some more skips for codespell

* [DATALAD RUNCMD] Do interactive fixing of some ambigous typos

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w -i 3 -C 2 ./community/front-end/ofe/script/service_account.sh ./community/front-end/ofe/website/ghpcfe/models.py ./community/front-end/ofe/website/ghpcfe/models.py ./community/modules/scripts/htcondor-install/files/autoscaler.py ./community/modules/scripts/ramble-setup/README.md ./docs/videos/healthcare-and-life-sciences/README.md ./examples/README.md ./tools/validate_configs/test_configs/README.md",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^

* 1 more typo fixed manually

* Skip (S)hortcuts, more words and files

* [DATALAD RUNCMD] run codespell throughout fixing typo automagically

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^

* Duplicate ignore of requirements.txt and js in pre-commit config

until codespell-project/codespell#3196 is addressed
ek-nag pushed a commit to ek-nag/hpc-toolkit that referenced this pull request Dec 8, 2023
* Add rudimentary codespell config

* Add pre-commit definition for codespell

* ot -> it typo fix

* Some more skips for codespell

* [DATALAD RUNCMD] Do interactive fixing of some ambigous typos

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w -i 3 -C 2 ./community/front-end/ofe/script/service_account.sh ./community/front-end/ofe/website/ghpcfe/models.py ./community/front-end/ofe/website/ghpcfe/models.py ./community/modules/scripts/htcondor-install/files/autoscaler.py ./community/modules/scripts/ramble-setup/README.md ./docs/videos/healthcare-and-life-sciences/README.md ./examples/README.md ./tools/validate_configs/test_configs/README.md",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^

* 1 more typo fixed manually

* Skip (S)hortcuts, more words and files

* [DATALAD RUNCMD] run codespell throughout fixing typo automagically

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^

* Duplicate ignore of requirements.txt and js in pre-commit config

until codespell-project/codespell#3196 is addressed
@nick-stroud nick-stroud mentioned this pull request Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-improvements Added to release notes under the "Improvements" heading.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants