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 pre-commit configuration (with codespell etc) to ensure no typos and more consistent formatting etc #11

Closed
wants to merge 4 commits into from

Conversation

yarikoptic
Copy link
Member

Relates to development procedures (hence #10)

TODOs if we agree to proceed

  • replace isort with hatch fmt or direct invocation to ruff? see Provide a pre-commit hook for hatch fmt pypa/hatch#1223
  • CI run to invoke pre-commit
    • cons in comparison to dedicated codespell action is that I do not think we would get annotations as we would get with using codespell-project/codespell-problem-matcher . So for codespell we could even have a dedicated CI run if no objections

may be

  • use black? (yet to compare to hatch fmt)

@yarikoptic
Copy link
Member Author

ah, me is yet to learn that commitizen format. also to come to pre-commit then

- repo: https://github.com/PyCQA/isort
rev: 5.12.0
hooks:
- id: isort
Copy link
Member

Choose a reason for hiding this comment

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

I cannot tell whether this is redundant or in conflict with the ruff setup.

Copy link
Member Author

Choose a reason for hiding this comment

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

as in original post TODOs - I think "yes" , we want to remove it after adding ruff formatting to pre-commit

- id: check-symlinks
- id: check-yaml
- id: end-of-file-fixer
- id: trailing-whitespace
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure what these check. But it would be essential for me to be able to run these checks locally, and not have to wait for the CI to tell me what is needed, and then have to rework a changeset again.

Copy link
Member Author

Choose a reason for hiding this comment

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

this config is for local operation really. https://pre-commit.com/

in a nut-shell

pip install pre-commit
pre-commit install # installs pre-commit git hook

and you are set -- it would invoke checks etc before commit.

Then if you want to run explicitly across entire codebase

pre-commit run --all

I do not think I ever user any other command there ;)

- repo: https://github.com/codespell-project/codespell
rev: v2.0.0
hooks:
- id: codespell
Copy link
Member

Choose a reason for hiding this comment

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

I have need instructions for running this locally somewhere.... Need to go searching.

Copy link
Member Author

Choose a reason for hiding this comment

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

codespell

config would be added to pyproject.toml

Copy link
Member Author

@yarikoptic yarikoptic Sep 21, 2024

Choose a reason for hiding this comment

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

IIRC could also be via pre-commit run --all codespell so it takes into account possible pre-commit specific configs (excludes) and operate only on files under git control (I think)

Copy link
Member Author

@yarikoptic yarikoptic Sep 21, 2024

Choose a reason for hiding this comment

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

added config, made it fix one typo it found , rewrote commit messages for prior commits. Let's see what cz thinks about datalad run ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

oh, you added config already, will rebase/adjust

mih added a commit that referenced this pull request Sep 20, 2024
An alternative for CI integration has be proposed (see refs). This adds
the necessary setup to configure and run this locally.

Two commands are provided (both discoverable via `hatch env show`:

- `codespell:check` (what would be executed by a CI)
- `codespell:fix` (to apply suggested fixes to the sources)

Refs: #11
mih added a commit that referenced this pull request Sep 21, 2024
An alternative for CI integration has be proposed (see refs). This adds
the necessary setup to configure and run this locally.

Two commands are provided (both discoverable via `hatch env show`:

- `codespell:check` (what would be executed by a CI)
- `codespell:fix` (to apply suggested fixes to the sources)

Refs: #11
@mih
Copy link
Member

mih commented Sep 21, 2024

ah, me is yet to learn that commitizen format. also to come to pre-commit then

This is not a commitizen format. This tool is just used to enforce https://www.conventionalcommits.org

skip = ".git,build,.*cache,dist"
check-hidden = true
# ignore-regex = ''
# ignore-words-list = ''
Copy link
Member Author

Choose a reason for hiding this comment

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

these two are most commonly used so I prefer to keep them present/ready.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (4ba0ecf) to head (1ef4677).

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #11   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines            6         6           
=========================================
  Hits             6         6           
Flag Coverage Δ
100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mih
Copy link
Member

mih commented Oct 8, 2024

I am closing this PR due to inactivity. Feel free to reopen once this work is completed.

@mih mih closed this Oct 8, 2024
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