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 hooks to the repo #212

Merged
merged 38 commits into from
Jul 26, 2023
Merged

Add pre-commit hooks to the repo #212

merged 38 commits into from
Jul 26, 2023

Conversation

TomTomRixRix
Copy link
Collaborator

@TomTomRixRix TomTomRixRix commented Apr 5, 2023

This PR installs the pre-commit hooks mentioned in .pre-commit-config.yaml. It also adds new GitHub Actions.

🔶 pre-commit hooks

  • checks yaml files for validity
  • checks if file over 500 kB were added
  • checks formatting with autopep8 and if necessary formats code (which then needs to be staged so that the hook passes the next time). Config may be adjusted in pyproject.toml under [tool.autopep8]
  • checks cff files for validity
  • inserts license header specified at license_header.txt into python files if there was none
  • checks the commit message (currently only ensures that there is one being at least 5 chars long) with gitlint. Configurations may be adjusted in .gitlint file
  • checks links in markdown files if they work or are dead (except those in the docs folder which might contain relative paths)
  • checks git config user email (optional) if it matches a pattern specified in a script which needs to be placed in the .git folder. This hook is commented out by default but can be activated with instructions given in the README

:octocat: GitHub Actions

  • run tests weekly every Friday morning at 6am
  • allow manuall triggering of tests
  • pushlish to PyPI on push to main

🚀 Misc:

  • added config for dependabot to run weekly and check for security issues of pip packages and github actions

Note that some commits were only done to check that the hooks work properly. so squash merging this PR might make sense.

Partly addresses issue #165 and solves #193

Steps to be done before merging:

  • Create summary of pre-commit hooks and actions here and use it to inform the developers
  • Run initial autopep8 formatting on all files (at the end to not mess up PR with too many changed files)

To be tested during review:

  • we might want to adjust the autopep8 config in the pyproject.toml
  • are there further pre-commit hooks that we might want to add?
  • is the current gitlint config sufficient?
  • do the weekly actions work?
  • does the publishing to PyPI work

@TomTomRixRix TomTomRixRix self-assigned this Apr 5, 2023
@TomTomRixRix TomTomRixRix marked this pull request as ready for review April 6, 2023 17:50
README.md Outdated
Comment on lines 71 to 99
## Install pre-commit hooks

In your SIMPA root directory run `pre-commit install` to set up the pre-commit hooks defined in `.pre-commit-config.yaml`. This will generate pre-commit hooks in `.git/hooks/` and run them for every commit. To run them manually before you commit, call `pre-commit run --all-files`.

<details>
<summary>Further information on pre-commit hooks</summary>

### git config user email address checking
The `git-config-email-check` hook is by default commented out in `pre-commit-config.yaml`. If you want to get a warning when you are not using a specified git config user email address domain, then you might want to comment this hook in. Then you would have to place a file according to the given path, e.g. at `.git/hooks/check-email.sh` with the following content:

```shell
#!/bin/bash
PWD=`pwd`
EMAIL=$(git config user.email)
if [[ $EMAIL == *"@yourdomain.com"* ]]; then
echo "[INFO] Verified email: $EMAIL"
else
echo "[ERROR] Invalid email: $EMAIL => Please configure the company email and retry."
echo "Steps:"
echo " cd $PWD"
echo ' git config user.email "<user>@yourdomain.com"'
echo ""
exit 1;
fi;
```

Change the domain in lines 4 and 10 according to your specific domain (e.g. corporate email domain) or even whole email address.
</details>

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should add this in the CONTRIBUTING.md file instead of the README.md

VERSION Outdated
Comment on lines 0 to 1
0.8.11 No newline at end of file
0.8.12
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this file anymore?

@kdreher kdreher self-requested a review July 26, 2023 09:16
kdreher
kdreher previously approved these changes Jul 26, 2023
@kdreher kdreher assigned kdreher and TomTomRixRix and unassigned TomTomRixRix and kdreher Jul 26, 2023
Copy link
Collaborator

@kdreher kdreher left a comment

Choose a reason for hiding this comment

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

Looks very good, great work, thank you! :) @TomTomRixRix

@kdreher kdreher merged commit e1d5a5b into develop Jul 26, 2023
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