Skip to content
Snippets Groups Projects

Add pre-commit hooks

Merged Ashwin Kumar Karnad requested to merge add-pre-commit-hook into main
2 unresolved threads

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
development.rst 0 → 100644
1 Developers Setup
  • This is an RST file (not markdown).

  • I didnt want to add an md file when the readme is in rst. Is there a reason why you chose to work on rst ( given that it has a strange syntax for a markup language)

  • RST is different from markdown, and you have learned markdown first, so perhaps that is why you consider it strange ;-). I agree the links are more intuitive in markdown.

    RST is more powerful, and the language used in sphinx and (often?) python docstrings.

    One feature I like in particular for README.rst files is that you can have a table of content created (which is not supported by markdown).

    I am all in favour of sticking to RST. I think I have fixed most of this file already?

  • Please register or sign in to reply
  • Hans Fangohr
  • Looks fine to me; apart from requested changes.

    It is an extra step to set up the repo for devolpment. So it is good this is documented. Should check that every possible developer is familiar with this (in Python).

  • added 1 commit

    • 5bf66265 - Apply changes from Hanss changes

    Compare with previous version

  • added 1 commit

    • 9c609105 - remove unused file checks in precommit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • If we assume every developer runs the pre-commit hooks, then no violation of that should ever be committed. But of course we cannot enforce that.

    How about we add a new workflow (or a command to our existing style checks) that runs the pre-commit checks and fails if it detects a violation? This way, we maintain the standard even if somebody doesn't use the pre-commit hooks. (As a downside: if you don't want to install the pre-commit hooks, and then the CI complains, then it is getting increasingly harder to fix this. On the positive: this will prevent that violations of the standards are detected by the next developer. )

  • Ok so ill replace the current style ci with pre-commit checks ( since it includes ruff and black )?

  • If all the checks are covered by the new pre-commit, and the output is as useful (I think it should be by default but you may want to check), then that seems fine. Perhaps do this in a new MR?

  • sure, ill do it once this is merged

  • added 2 commits

    • e4061935 - remove reaquirements txt check
    • cfd060a7 - run precommit on all files

    Compare with previous version

  • added 4 commits

    Compare with previous version

  • Hans Fangohr approved this merge request

    approved this merge request

  • merged

  • Hans Fangohr mentioned in commit b322f8f3

    mentioned in commit b322f8f3

  • mentioned in issue #17 (closed)

  • Please register or sign in to reply
    Loading