-
Notifications
You must be signed in to change notification settings - Fork 117
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
Merge branch dev/pre-commit-hook #156
Open
gierle
wants to merge
4
commits into
automl:Develop
Choose a base branch
from
gierle:dev/pre-commit-hook
base: Develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
[general] | ||
contrib = contrib-title-conventional-commits | ||
ignore = body-is-missing | ||
|
||
[title-max-length] | ||
line-length=72 | ||
|
||
[body-max-line-length] | ||
line-length=100 | ||
|
||
[contrib-title-conventional-commits] | ||
# Specify allowed commit types. For details see: ./CONTRIBUTING.md | ||
types = feat,fix,docs,refactor,perf,style,build,test,chore,revert |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,34 +1,54 @@ | ||
exclude: ^docs/ | ||
repos: | ||
- repo: https://github.com/pre-commit/mirrors-mypy | ||
rev: v0.761 | ||
# Check and update general problems | ||
- repo: https://github.com/pre-commit/pre-commit-hooks | ||
rev: v4.4.0 | ||
hooks: | ||
- id: mypy | ||
args: [--show-error-codes, | ||
--warn-redundant-casts, | ||
--warn-return-any, | ||
--warn-unreachable, | ||
- id: check-yaml # Attempts to load all yaml files to verify syntax. | ||
- id: debug-statements # Check for debugger imports and breakpoint() calls in python source. | ||
- id: end-of-file-fixer # Makes sure files end in a newline and only a newline. | ||
- id: trailing-whitespace # Trims trailing whitespace. | ||
args: [ --markdown-linebreak-ext=md ] # preserve Markdown hard linebreaks | ||
exclude: ^tests/ | ||
|
||
# Reordering Imports (similar to isort) | ||
# This style is mainly focussed on merge conflicts, see: | ||
# https://github.com/asottile/reorder_python_imports#why-this-style | ||
- repo: https://github.com/asottile/reorder_python_imports | ||
rev: v3.9.0 | ||
hooks: | ||
- id: reorder-python-imports | ||
args: [ | ||
# --application-directories, '.:src', # assumes project rooted at . | ||
--py37-plus, | ||
# --add-import, 'from __future__ import annotations', # unsure if required | ||
] | ||
files: naslib/.* | ||
exclude: | ||
- naslib/examples/.* | ||
- naslib/docs/.* | ||
|
||
- repo: https://gitlab.com/pycqa/flake8 | ||
rev: 3.8.3 | ||
|
||
# Automatically converts syntax to specified version | ||
- repo: https://github.com/asottile/pyupgrade | ||
rev: v3.3.1 | ||
hooks: | ||
- id: pyupgrade | ||
args: [--py37-plus] | ||
|
||
# Code formatter | ||
- repo: https://github.com/psf/black | ||
rev: 22.12.0 | ||
hooks: | ||
- id: black-jupyter # supports .py and .ipynb files | ||
args: [--line-length=79, --target-version=py37] | ||
|
||
# Linting | ||
- repo: https://github.com/PyCQA/flake8 | ||
rev: 6.0.0 | ||
hooks: | ||
- id: flake8 | ||
|
||
# Type Checking | ||
# According to this blog post (https://jaredkhan.com/blog/mypy-pre-commit) | ||
# one should consider running mypy on all files rather than on only the committed changes | ||
# since the changes can break dependencies elsewhere (e.g. function naming) | ||
- repo: https://github.com/pre-commit/mirrors-mypy | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. Cleaning up the entire repo with mypy is an item on our list. We should enforce mypy in pre-commit hooks only once we've done that. |
||
rev: v0.991 | ||
hooks: | ||
- id: flake8 | ||
additional_dependencies: | ||
- flake8-print==3.1.4 | ||
- flake8-import-order | ||
name: flake8 naslib | ||
files: naslib/.* | ||
exclude: | ||
- naslib/examples/.* | ||
- naslib/docs/.* | ||
- naslib/predictors/.* | ||
- id: flake8 | ||
additional_dependencies: | ||
- flake8-print==3.1.4 | ||
- flake8-import-order | ||
name: flake8 test | ||
files: tests/.* | ||
- id: mypy |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
# Contributing to NASLib | ||
|
||
- [Tools](#tools) | ||
- [Pre-Commit Hooks](#pre-commit-hooks) | ||
- [Gitlint](#gitlint) | ||
- [Styleguide](#styleguide) | ||
- [Commit Message Guidelines](#commit-message-format) | ||
|
||
## Tools | ||
|
||
The listed tools here are meant to be supportive for you as a developer and help as guidance to submit good quality code. However, they are optional and not developed by ourselves. | ||
|
||
### Pre-Commit Hooks | ||
|
||
Git hook scripts are useful for identifying simple issues before submission to code review. Pre-commit hooks run before committing and are used for fixing minor problems, linting or type checking. See [.pre-commit-config.yaml](/.pre-commit-config.yaml) for our configuration and the [pre-commit website][pre-commit] for further information. | ||
|
||
Installation: | ||
|
||
```bash | ||
pip install pre-commit==3.0.3 | ||
pre-commit install | ||
``` | ||
|
||
### Gitlint | ||
|
||
A commit message linter, which enforces some rules of the defined [commit message format](#commit-message-format). You can find further information on [their website][gitlint] as well as [our configuration](/.gitlint). | ||
It can be used as `commit-msg hook`, which prevents you from committing using the wrong format: | ||
|
||
```bash | ||
pip install gitlint==0.18.0 | ||
gitlint install-hook | ||
# gitlint uninstall-hook | ||
``` | ||
|
||
## Styleguide | ||
|
||
### Commit Message Format | ||
|
||
*This specification is inspired by the commit message format of [AngularJS][angular-commits] as well as [conventional commits][conventional-commits].* | ||
|
||
#### TL:DR | ||
|
||
- Use the present tense (*"add feature"* not *"added feature"*) | ||
- Use the imperative mood (*"move cursor to..."* not *"moves cursor to..."*) | ||
- Commit header consists of a [`type`](#types)) and an uncapitalized [`summary`](#subject) | ||
- Limit the header to 72 characters | ||
- Limit any line of the commit message to 100 characters | ||
- Motivate the change in the `body` | ||
- Reference issues and pull requests in your footer | ||
|
||
|
||
#### More in-depth | ||
|
||
Each commit message consists of a **header**, an optional [**body**](#body), and an optional [**footer**](#footer), separated by a blank line each. | ||
The message header consists of a [**type**](#types), an optional [**scope**](#scopes) and a [**subject**](#subject). | ||
|
||
The header is limited to 72 characters, while any other line cannot exceed 100 characters. This allows for better readability in GitHub as well as respective tools. | ||
|
||
``` | ||
<type>(<optional scope>): <subject> | ||
<BLANK LINE> | ||
<optional body> | ||
<BLANK LINE> | ||
<optional footer(s)> | ||
``` | ||
|
||
##### Types | ||
|
||
| Type | Description | | ||
| --- | --- | | ||
|`feat` | Commits, which add new features | | ||
|`fix` | Commits, which fix a bug | | ||
|`docs` | Commits, that only affect documentation | | ||
|`refactor` | Commits, which change the stucture of the code, however do not change its behaviour | | ||
|`perf` | Special `refactor` commits, which improve performance | | ||
|`style` | Commits, that do not affect the meaning (white-space, formatting, missing semi-colons, etc) | | ||
|`build` | Commits, that affect build components like build tool, ci pipeline, dependencies, project version, ... | | ||
|`test` | Commits, that add missing tests or correcting existing tests | | ||
|`chore` | Miscellaneous commits e.g. modifying `.gitignore` | | ||
|`revert`| see chapter [Revert](#revert) | | ||
|
||
##### Scopes | ||
|
||
Scopes are project specific and will be defined later. | ||
|
||
##### Subject | ||
|
||
The `subject` contains a succinct description of the change. | ||
|
||
- Use the imperative, present tense: "change" not "changed" nor "changes". | ||
- Think of `This commit will <subject>` | ||
- Don't capitalize the first letter | ||
- No dot (.) at the end | ||
|
||
##### Body | ||
|
||
The `body` should explain the motivation behind the change. It can include a comparison between new and previous behaviour. | ||
It can consist of multiple newline separated paragraphs. As in `subject`, use the imperative, present tense. | ||
It is an optional part. | ||
|
||
##### Footer | ||
|
||
The `footer` contains information of breaking changes and deprecation, references to e.g. GitHub issues or PRs as well as other metadata (e.g. [trailers](https://git.wiki.kernel.org/index.php/CommitMessageConventions)). | ||
To separate footer from the body, especially for parsing, the [specifications 8. - 10. of conventional commit](https://www.conventionalcommits.org/en/v1.0.0/#specification) exist<sup>[1](#footnote1)</sup>. | ||
|
||
``` | ||
{BREAKING CHANGE, DEPRICATED}: <summary> | ||
<description + migration instructions or update path> | ||
<BLANK LINE> | ||
Close #<id> | ||
Reviewed-by: xyz | ||
``` | ||
|
||
##### Revert | ||
|
||
If the commit reverts a previous commit, its header should begin with `revert: `, followed by the header of the reverted commit. | ||
In the body it should say: `This reverts commit <hash>.`, where the hash is the SHA of the commit being reverted. | ||
|
||
___ | ||
|
||
<a name="footnote1">1</a>: Important note: `BREAKING CHANGE` is the only exception for specification 9., where a `<space>` between words is not replaced by a `-` | ||
|
||
[angular-commits]: https://docs.google.com/document/d/1QrDFcIiPjSLDn3EL15IJygNPiHORgU1_OOAqWjiDU5Y/edit# | ||
[conventional-commits]: https://www.conventionalcommits.org/en/v1.0.0/ | ||
[pre-commit]: https://pre-commit.com/ | ||
[gitlint]: https://jorisroovers.com/gitlint/ |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optimal line length is a bitterly contested subject, but I personally find 79 to be too short. I've seen other projects that use 88. I'd much prefer that.