Skip to content

Fix typos and add codespell to nox#45

Merged
flozz merged 1 commit intowanadev:masterfrom
myfloss:fix-typos-and-add-nox-session
Oct 18, 2022
Merged

Fix typos and add codespell to nox#45
flozz merged 1 commit intowanadev:masterfrom
myfloss:fix-typos-and-add-nox-session

Conversation

@kianmeng
Copy link
Copy Markdown
Contributor

No description provided.

@flozz
Copy link
Copy Markdown
Member

flozz commented Oct 17, 2022

Hello,

Thank you for your PR!

I do not know if it is a good idea to run codespell in the lint task but we will see if it causes problem by using it. :)

My only problem is that we should list explicitly what to check instead of just skipping .nox folder. For example if you pull the submodules, the assimp/ folder (that is a third party dependency) will be spell checked and will cause many errors to be reported. Locally I also have build/, dist/, __env__/, etc. folders that should not be checked. Maybe we can change the codespell call to:

    session.run(
        "codespell",
        "-L",
        "ans,alph",
        "doc/",
        "scripts/",
        "test/",
        "winbuild/",
        "yoga/",
        "noxfile.py",
        "README.rst",
        "setup.py",
    )

Also, can you add codespell in the dev dependencies in the setup.py file?

    extras_require={
        "dev": [
            "nox",
            "flake8",
            "black",
            "pytest",
            "Sphinx",
            "sphinx-rtd-theme",
            "codespell",
        ]
    },

@kianmeng kianmeng force-pushed the fix-typos-and-add-nox-session branch from 5fdf053 to 1986a95 Compare October 17, 2022 16:29
@kianmeng
Copy link
Copy Markdown
Contributor Author

Requested changes updated.

@flozz flozz merged commit 48fafc3 into wanadev:master Oct 18, 2022
@flozz
Copy link
Copy Markdown
Member

flozz commented Oct 18, 2022

Thank you for the fixes 👍

@kianmeng
Copy link
Copy Markdown
Contributor Author

🥳 🥳 🥳 🥳 🥳

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.

2 participants