Update Contributing Guide (#832)
* Update Contributing Guide * Minor improvements to README and CONTRIBUTING
This commit is contained in:
committed by
GitHub
parent
75ddafd31d
commit
df8e3dd82b
+72
-61
@@ -1,27 +1,48 @@
|
|||||||
# Contributing Guide
|
# Contributing Guide
|
||||||
|
|
||||||
When contributing to this repository, please first discuss the change you wish to make via the
|
When contributing to this repository, please first discuss the change you wish to make via the
|
||||||
issue tracker or the discussions page with the owner of this repository before making a change. If
|
issue tracker or the discussions page with the owner of this repository. Especially if it is
|
||||||
you just want to make a minor correction, like fix a typo or similar, feel free to just make a pull
|
regarding new features. If you just want to make a minor correction, like fix a typo or similar,
|
||||||
request directly.
|
feel free to just make a pull request directly.
|
||||||
|
|
||||||
## Branching Structure
|
The following contributions are welcome:
|
||||||
|
|
||||||
This project mainly follows the OneFlow branching structure. The `main` branch is the default
|
* Bugfixes for new or existing bugs. Please also report new bugs in the issue tracker even if you
|
||||||
branch. If your contribution is a bugfix for a specific version, please make a bugfix branch from
|
also provide a fix. It makes it easier to keep track of what has been fixed and when.
|
||||||
the tag you want the fix to be applied to. For other changes, make a feature branch from the
|
* New translations. You can read more about contributing translations
|
||||||
current `main` branch.
|
[here](https://github.com/vkbo/novelWriter/blob/main/i18n/README.md).
|
||||||
|
* Improvements to the documentation. Particularly if the documentation is unclear. Please don't
|
||||||
|
make any larger changes to the documentation without discussing if with the maintainer first.
|
||||||
|
* Adaptations, installation or packaging features targeting specific operating systems.
|
||||||
|
|
||||||
## Pull Request Process
|
Please do not:
|
||||||
|
|
||||||
1. Make sure your code passes all tests and conforms to the style guide. You can check that the
|
* Make a pull request that restructures or reformats existing code. If you think some parts of the
|
||||||
code conforms by running `flake8` from the root of the project folder. The same check is also
|
code could be improved, please make an issue thread or start a discussion. The same applies to
|
||||||
run on pull requests by the maintainer.
|
any text document in the repository.
|
||||||
2. Please provide a complete description of the changes in the pull request, and a summary that can
|
|
||||||
be copied into the [CHANGELOG](CHANGELOG.md). Remember to reference any issue related by
|
## Pull Requests
|
||||||
providing the issue number.
|
|
||||||
3. Do not change the version number. Version numbers are bumped in separate release pull requests
|
This project follows the [OneFlow](https://www.endoflineblog.com/oneflow-a-git-branching-model-and-workflow)
|
||||||
by the maintainer.
|
model. The `main` branch is the default branch. For general changes, please make a new branch in
|
||||||
|
your own fork from the current `main` branch. Do not make pull requests from your own `main`
|
||||||
|
branch.
|
||||||
|
|
||||||
|
**Note:** If your contribution is a bugfix for a specific version, please make a bugfix branch from
|
||||||
|
the tag you want the fix to be applied to, not directly from the `main` branch. This is important.
|
||||||
|
|
||||||
|
Also check the following:
|
||||||
|
|
||||||
|
* Make sure your code passes all tests and conforms to the style guide. You can check that the
|
||||||
|
code generally conforms by running the Python linting tool `flake8` from the root of the project
|
||||||
|
folder, although it doesn't check everything. The same check is also run on pull requests by the
|
||||||
|
maintainer.
|
||||||
|
* Please provide a description of the changes in the pull request under the summary section of the
|
||||||
|
pull request template, and reference any related issues by providing the issue number.
|
||||||
|
* Do not change the version number.
|
||||||
|
* Do not submit files that were not actively changed but have otherwise been modifed. This is
|
||||||
|
mostly an issue with translation files. The language tool may update all files in the `i18n`
|
||||||
|
folder.
|
||||||
|
|
||||||
## Code of Conduct
|
## Code of Conduct
|
||||||
|
|
||||||
@@ -40,29 +61,42 @@ Please see the [CODE_OF_CONDUCT](CODE_OF_CONDUCT.md) file for the full text.
|
|||||||
## Code Style Guide
|
## Code Style Guide
|
||||||
|
|
||||||
The source code of novelWriter broadly follows the [PEP8](https://www.python.org/dev/peps/pep-0008)
|
The source code of novelWriter broadly follows the [PEP8](https://www.python.org/dev/peps/pep-0008)
|
||||||
style guide, but with a few modifications and exceptions listed below.
|
style guide, but with a few exceptions. Some key points are listed below.
|
||||||
|
|
||||||
### Line Length
|
**Line Length:**
|
||||||
|
|
||||||
For this project, source lines should stay within the 79 and 99 character limits described by PEP8.
|
* Source code lines can extend to the upper limit of 99 characters allowed by PEP8. 79 characters
|
||||||
79 characters is often too restrictive, so 99 character lines are acceptable when that is more
|
is the recommended line length in PEP8, but this is often too restrictive. 99 characters are
|
||||||
practical. Readability has priority. Generally, if a code statement requires multiple lines, the
|
acceptable when that is more practical. Readability has priority. Generally, if a code statement
|
||||||
lines should wrap at 79 characters, not 99. If wrapping can be avoided by going to 99, then that is
|
requires multiple lines, the lines should wrap at 79 characters if possible. If wrapping can be
|
||||||
generally preferrable.
|
avoided by going to 99, then that is generally preferrable.
|
||||||
|
* For text files, the text should be wrapped at 99 character. The exception is markdown image tags
|
||||||
|
and urls which can run past that limit.
|
||||||
|
|
||||||
For text files, the text should also be wrapped at 99 character. The exception is markdown image
|
**Variable and Function Names**
|
||||||
tags and urls which can run past that limit.
|
|
||||||
|
|
||||||
Please do not submit pull requests that re-wrap existing source or text unless this has been
|
* PEP8 allows for camelCase for consistency with existing code. The Qt library uses camelCase, so
|
||||||
discussed beforehand.
|
the Python the source code does too.
|
||||||
|
* The exception to the above is for constants. They should always be in upper snake case, like PEP8
|
||||||
|
states.
|
||||||
|
|
||||||
|
**Spaces, Indentation and Alignment**
|
||||||
|
|
||||||
|
* Only indentation by 4 spaces is allowed.
|
||||||
|
* No trailing spaces should occur on any line in the source code, including empty lines.
|
||||||
|
* Ideally, a function should end on the same indention level as it started. Exceptions are allowed
|
||||||
|
if it makes the code easier to follow.
|
||||||
|
* Aligning operators and attributes in columns with multiple spaces is not allowed by PEP8. The
|
||||||
|
rule is relaxed a bit here. Alignment is allowed when populating large dictionaries or setting
|
||||||
|
many class attributes. It does improve readability in such cases, but should not be overused.
|
||||||
|
|
||||||
### Linting with `flake8`
|
### Linting with `flake8`
|
||||||
|
|
||||||
An excellent tool for checking Python code for errors and coding style is `flake8`. The
|
A good tool for checking Python code for errors and code style is `flake8`. The documentation is
|
||||||
documentation is available [here](https://flake8.pycqa.org/en/latest/).
|
available [here](https://flake8.pycqa.org/en/latest/).
|
||||||
|
|
||||||
The `setup.cfg` file in the root of this project has the following settings for `flake8` that
|
The `setup.cfg` file in the root of this project has the following settings for `flake8` that
|
||||||
matches the coding standard:
|
matches the above code style:
|
||||||
```conf
|
```conf
|
||||||
[flake8]
|
[flake8]
|
||||||
ignore = E221,E226,E228,E241
|
ignore = E221,E226,E228,E241
|
||||||
@@ -75,38 +109,15 @@ The command line equivalent, with reporting, is:
|
|||||||
flake8 . --count --ignore E221,E226,E228,E241 --max-line-length=99 --show-source --statistics
|
flake8 . --count --ignore E221,E226,E228,E241 --max-line-length=99 --show-source --statistics
|
||||||
```
|
```
|
||||||
|
|
||||||
Passing this check is required before contributions are merged into the `main` branch. This is
|
Passing this check is required before contributions are merged. This is checked automatically when
|
||||||
checked automatically when you make a pull request. You can run the `flake8` command locally to
|
you make a pull request. You can run the `flake8` command locally to check beforehand. The full
|
||||||
check beforehand. The full command will give you a detailed description of the code lines that do
|
command will give you a detailed description of the code lines that do not conform to the standard.
|
||||||
not conform to the standard.
|
|
||||||
|
|
||||||
### Ignored Errors
|
Two of the ignored errors are due to the relaxed restriction on column alignment, these are the
|
||||||
|
E221 and E241 error codes.
|
||||||
|
|
||||||
Some `flake8` error codes are ignored for this project for various reasons. The source also uses
|
The code E226 is ignored becuse it doesn't actually follow the
|
||||||
camelCase function and variable names. This is the standard for the Qt libraries novelWriter
|
|
||||||
integrates with. It also happens to be the author's personal preferences. (Yay!)
|
|
||||||
|
|
||||||
The reason behind the other ignored error codes are listed below. Two of them are due to PEP8 not
|
|
||||||
permitting column alignment as opposed to many other coding styles, like for instance for Go. I
|
|
||||||
find them useful in regions of bulk value assignments. There's a reason why tables are more
|
|
||||||
readable than lists. They should be used sparingly though. If you're unsure, follow PEP8.
|
|
||||||
|
|
||||||
The ignored errors are all `pycodestyle` errors, and they are documented
|
|
||||||
[here](https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes).
|
|
||||||
|
|
||||||
**E221:** multiple spaces before operator
|
|
||||||
**Reason:** Column alignment.
|
|
||||||
|
|
||||||
**E226:** missing whitespace around arithmetic operator
|
|
||||||
**Reason:** This doesn't actually follow the
|
|
||||||
[PEP8 recommendation](https://www.python.org/dev/peps/pep-0008/#other-recommendations)
|
[PEP8 recommendation](https://www.python.org/dev/peps/pep-0008/#other-recommendations)
|
||||||
of grouping longer equations by operator precedence like `2*a + 3*b` instead of `a * a + 3 * b`.
|
of grouping longer equations by operator precedence like `2*a + 3*b` instead of `a * a + 3 * b`.
|
||||||
Generally, don't use spaces around `*`, `/` and `**`, but _do_ use spaces around `+` and `-`.
|
Generally, don't use spaces around `*`, `/` and `**`, but _do_ use spaces around `+` and `-`.
|
||||||
For appending strings, the spaces can be dropped. Don't use the `+` operator for appending multiple
|
The same applies to the modulo operator `%`, hence E228 is also ignored.
|
||||||
strings. Use formatting instead.
|
|
||||||
|
|
||||||
**E228** missing whitespace around modulo operator
|
|
||||||
**Reason:** See reason for E226. Formatting `%` like `/` and `*` should be possible.
|
|
||||||
|
|
||||||
**E241:** multiple spaces after ‘,’
|
|
||||||
**Reason:** Column alignment.
|
|
||||||
|
|||||||
@@ -34,15 +34,15 @@ met. It is regularly tested on Debian and Ubuntu Linux, Windows, and macOS.
|
|||||||
|
|
||||||
Please don't make feature pull requests without first having discussed them with the maintainer.
|
Please don't make feature pull requests without first having discussed them with the maintainer.
|
||||||
You can make a feature request in the issue tracker, or if the idea isn't fully formed, start a
|
You can make a feature request in the issue tracker, or if the idea isn't fully formed, start a
|
||||||
discussion on the discussion page. Fixes and patches are welcome. So are new translations.
|
discussion on the discussion page. Please also don't make pull requests to reformat or rewrite
|
||||||
Contributions related to packaging and installing novelWriter will also be appreciated.
|
existing code unless there is a very good reason for doing so.
|
||||||
|
|
||||||
If you want to help translating novelWriter into another language, please see the
|
Fixes and patches are welcome. Contributions related to packaging and installing novelWriter will
|
||||||
[README](https://github.com/vkbo/novelWriter/blob/main/i18n/README.md) in the `nw/assets/i18n`
|
also be appreciated, but please make an issue or a discussion topic first. Before contributing any
|
||||||
folder for further details.
|
code, please read the full [Contributing Guide](https://github.com/vkbo/novelWriter/blob/main/CONTRIBUTING.md).
|
||||||
|
|
||||||
Before contributing any code, please read the full
|
New translations are always welcome. Please read the additional
|
||||||
[Contributing Guide](https://github.com/vkbo/novelWriter/blob/main/CONTRIBUTING.md).
|
[instructions](https://github.com/vkbo/novelWriter/blob/main/i18n/README.md) for further details.
|
||||||
|
|
||||||
## Key Features
|
## Key Features
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user