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

Fix typos found by codespell #1261

Merged
merged 1 commit into from
Mar 23, 2023
Merged

Fix typos found by codespell #1261

merged 1 commit into from
Mar 23, 2023

Conversation

DimitriPapadopoulos
Copy link
Contributor

No description provided.

@drammock
Copy link
Collaborator

@DimitriPapadopoulos want to add codespell to our CI config?

@DimitriPapadopoulos
Copy link
Contributor Author

I can do that if you're willing to live with the downsides:

  • false positives need to be silenced by a list of words to ignore
  • new false positives may appear all of a sudden because new typos have been added to codespell dictionaries, including in patch releases

@12rambau
Copy link
Collaborator

One of the typo you identifed was in kitchen sink which is inherited from the sphinx-theme website. I reported it upstream.

@drammock
Copy link
Collaborator

One of the typo you identifed was in kitchen sink which is inherited from the sphinx-theme website. I reported it upstream.

ooh, good catch. we shouldn't be modifying kitchen sink here.

Copy link
Collaborator

@12rambau 12rambau left a comment

Choose a reason for hiding this comment

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

instead of adding it to tests, would it be preferable to add it to the pre-commit ? they have a hook and this would make sure any commit is already checked + we are already checking the pre-commits in the tests

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Mar 23, 2023

Yes, probably better in pre-commit. Unfortunately I don't have much experience with pre-commit, because of previous bad experience with it (lots of compatibility issues I didn't have time to debug on my Ubuntu 18.04 machine). I'll give it a try.

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Mar 23, 2023

Among other annoyances, pre-commit doesn't work on Ubuntu 22.04 by default:

return code: 1
expected return code: 0
stdout:
    Executable `python` not found
stderr: (none)

Easily fixed with:

sudo apt install python-is-python3 

Buit then I get this new error:

An unexpected error has occurred: AssertionError: BUG: expected environment for python to be healthy() immediately after install, please open an issue describing your environment

I find pre-commit is a pain to use locally.

@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as ready for review March 23, 2023 20:18
@drammock
Copy link
Collaborator

let's roll back the CI addition and address adding codespell to pre-commit in a separate PR. No need to drag out the simpler changes. (please also revert the change to kitchen sink too, if you haven't already).

@DimitriPapadopoulos
Copy link
Contributor Author

I have reverted the CI/pre-commit stuff.

@drammock drammock merged commit 7a7087a into pydata:main Mar 23, 2023
@DimitriPapadopoulos DimitriPapadopoulos deleted the codespell branch March 24, 2023 06:45
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.

3 participants