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 discovered by codespell #68

Merged
merged 1 commit into from
Dec 11, 2024
Merged

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Dec 11, 2024

PR Goal?

Fix typos with https://pypi.org/project/codespell

% codespell --ignore-words-list=avance,delt --skip="dict.txt,*.pyx" --write-changes

Fixes?

Feedback sought?

Priority?

Tests added?

How to test?

Confidence?

Version change?

@joanise
Copy link
Member

joanise commented Dec 11, 2024

Thanks for submitting this PR! Yes, fixing the typos is a good idea.

@cclauss
Copy link
Contributor Author

cclauss commented Dec 11, 2024

Fixed at failing test? Modern OSes mandate a "managed" system Python where pip cannot install without a venv.

@joanise
Copy link
Member

joanise commented Dec 11, 2024

Fixed at failing test? Modern OSes mandate a "managed" system Python where pip cannot install without a venv.

Ah, I was wondering why this workflow did use the actions/setup-python, I guess it's a leftover from old CI files, because we use it everywhere else in our projects.

I thought it might have been related to our old use of requirements.dev.txt instead of a .[dev] install defined in pyproject.toml, but maybe not.

Hum, no, it's more complex: it's still failing, both your patch and my attempt in #69

@joanise
Copy link
Member

joanise commented Dec 11, 2024

BTW, if you want to exercise the CI in your fork without having to wait for my approval, you can create a PR from your branch to your own main and then you'll be able to troubleshoot on your side.

@joanise
Copy link
Member

joanise commented Dec 11, 2024

And thanks for your help here, but don't feel obligated, we'll figure it out.

@cclauss
Copy link
Contributor Author

cclauss commented Dec 11, 2024

pyproject.tomll: requires = [ "setuptools>=61.2" ]

@joanise
Copy link
Member

joanise commented Dec 11, 2024

pyproject.tomll: requires = [ "setuptools>=61.2" ]

Right, the was my next line of inquiry, where to force the setuptools version, since the error messages complain about the version.

@cclauss
Copy link
Contributor Author

cclauss commented Dec 11, 2024

I reverted all non-codespell changes.

@joanise
Copy link
Member

joanise commented Dec 11, 2024

Urgh, CI ran on the old merge commit, not on the new one with my PR merged. Grrr.

@joanise
Copy link
Member

joanise commented Dec 11, 2024

Approving anyway, we know this is fixed.

@joanise joanise merged commit e911154 into ReadAlongs:main Dec 11, 2024
4 of 5 checks passed
@cclauss cclauss deleted the codespell branch December 11, 2024 17:56
@dhdaines
Copy link
Contributor

Oh, thanks! I was wondering about that CI failure...

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