From df2b913573ee7d9e9285c4c46aee62e143405a72 Mon Sep 17 00:00:00 2001 From: miruka Date: Thu, 12 Nov 2020 10:41:58 -0400 Subject: [PATCH] Add CONTRIBUTING.md --- README.md | 6 +- docs/CONTRIBUTING.md | 292 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 297 insertions(+), 1 deletion(-) create mode 100644 docs/CONTRIBUTING.md diff --git a/README.md b/README.md index 7739aab4..1403cba9 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,7 @@ [Features](#currently-implemented-features) ⬥ [Installation](docs/INSTALL.md) ⬥ [Configuration & Theming](#configuration--theming) ⬥ +[Contributing](docs/CONTRIBUTING.md) ⬥ [Screenshots](#more-screenshots) A fancy, customizable, keyboard-operable [Matrix](https://matrix.org/) chat @@ -155,6 +156,10 @@ restarting the app. GUI settings will also be implemented in the future. +## Contributing + +See [CONTRIBUTING.md](docs/CONTRIBUTING.md) + ## Screenshots ![Sign-in](docs/screenshots/02-sign-in.png) @@ -164,4 +169,3 @@ GUI settings will also be implemented in the future. ![Main pane in small window](docs/screenshots/05-main-pane-small.png) ![Chat in small window](docs/screenshots/06-chat-small.png) ![Room pane in small window](docs/screenshots/07-room-pane-small.png) - diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md new file mode 100644 index 00000000..ef448c0f --- /dev/null +++ b/docs/CONTRIBUTING.md @@ -0,0 +1,292 @@ +# Contributing + +- [Issues](#issues) +- [Pull Requests](#pull-requests) + - [Procedure](#procedure) +- [Commit Guidelines](#commit-guidelines) +- [Coding Conventions](#coding-conventions) + - [General](#general) + - [Python](#python) + - [QML](#qml) + - [C++](#c) +- [Resources](#resources) +- [Packaging](#packaging) + +## Issues + +[Issues](https://github.com/mirukana/mirage/issues) on GitHub should be used to +ask questions, report problems, request new features, +or discuss potential changes before creating pull requests. + +Before opening new issues, please search for any already open or closed +issue related to your problem, in order to prevent duplicates. + +You can also join us on the +[#mirage-client:matrix.org](https://matrix.to/#/%23mirage-client:matrix.org) +room for questions and discussions. + + +## Pull Requests + +For changes outside of simple bug/typo/formatting fixes, it is strongly +recommended to first discuss your ideas in a related issue or in +[#mirage-client:matrix.org](https://matrix.to/#/%23mirage-client:matrix.org). + +New changes are merged to the +[`dev` branch](https://github.com/mirukana/mirage/tree/dev) first. +Once a new version of the application is released, +the current `dev` is merged into the main `master` branch. + +By sending your changes, you agree to license them under the LGPL 3.0 or later. + +### Procedure + +Start by forking the main repository from GitHub, then +clone your fork and switch to a new branch based on `dev`, in which +you will make your changes: + +```sh +git clone --recursive https://github.com/yourUsername/mirage +cd mirage +git remote add upstream https://github.com/mirukana/mirage +git fetch upstream +git checkout upstream/dev +git branch example-branch-name +git checkout example-branch-name +``` + +Test and commit your changes according to the +[commit guidelines](#commit-guidelines), and `git push` to your fork's repo. +You will then be able to make a pull request by going +to the [main repo](https://github.com/mirukana/mirage). + +Once your pull request is merged, you can update `dev`, and delete your +GitHub and local branch: + +```sh +git fetch upstream +git checkout upstream/dev + +git push -d origin example-branch-name +git branch -d example-branch-name +``` + +Make sure `dev` is up-to-date before creating new branches based on it. + + +## Commit Guidelines + +Commit messages should be made in this form: + +``` +Title, a short summary of the changes + +The body, a more detailed summary needed depending on the changes. +Explain the goal of the code, how to reproduce the bug it solves +(if this is a bug fix), any special reasoning behind the +implementation or side-effects. +``` + +- Write informative titles, e.g. + `TextField: fix copying selected text by Ctrl+C` instead of + `fix field bug` + (assuming `TextField` was the name of the component affected by the bug) + +- Write the title in imperative form and without a period at the end, + e.g. `Fix thing` instead of `Fixed thing` or `Fixes thing.` + +- The title must not exceed 50 characters + +- A blank line is required between the first line summary and detailed + body, if there is one + +- Lines of the body must not exceed 72 characters + +- Split independent changes into separate commits, + don't combine fixes for different problems or add multiple systems forming a + complex feature all at once + +- Every commit should be able to build and run the application without + obvious crashes or tracebacks + +- Check for linter errors before committing by running `make test` in the + repository's root. The test tools can be installed with + `pip3 install --user -Ur requirements-dev.txt`. + +- For changes that aren't yet merged in a branch of the main repo, + prefer amending or editing previous commits via git interactive rebase, + rather than adding new "fix this" commits. + This helps keeping the history clean. + + +## Coding Conventions + +### General + +- Use four space indentations, no tabs + +- Use double quotes for strings, unless single quotes would avoid having to + escape quotes in the text + +- Keep lines under 80 columns, the only exception for this is long URL links + that can't be broken in multiple parts + +- Keep lines free from any trailing whitespace + +- For function definitions, calls and list/dict/etc not fitting in + one line, keep a trailing comma on the last element: + + ```python3 + short_function(1, 2, 3) + + long_function( + long_argument_1, long_argument_1, long_argument_3, long_argument_4, # 🡐 + ) + + large_list = [ + "some long piece of text here number 1", + "some long piece of text here number 2", + "some long piece of text here number 3", # 🡐 + ] + ``` + +- When creating new files, don't forget the copyright and license + header you see in other files of the same language. + +### Python + +- All functions, class attributes or top-level variables should have type hints + +- Separate all top-level classes and functions by two blank lines. + For classes with many long methods, separate those methodes by two blank + lines too. + +- Readability is important. Vertically align consecutive lines of assignments, + function definition arguments, dictionaries and inline comments: + + ```python3 + # Bad: + + num: int = 1 # A comment + args: List[str] = ["a", "b"] # Another comment + + def func( + self, + example_argument: int = 300, # Comment + other: str = "Sample text", # Other comment + some_floats: Tuple[float, float, float] = (4.2, 1.1, 9.8), + xyz: bool = False, + things_set: Optional[Set[str]] = None, + ) -> None: + pass + + # Good: + + num: int = 1 # A comment + args: List[str] = ["a", "b"] # Another comment + + def func( + self, + example_arg: int = 300, # Comment + other: str = "Sample text", # Other comment + some_floats: Tuple[float, float] = (4.2, 9.8), + xyz: bool = False, + things_set: Optional[Set[str]] = None, + ) -> None: + pass + ``` + + If this is annoying, consider getting a plugin for your editor to automate it + (e.g. [EasyAlign](https://github.com/junegunn/vim-easy-align) for vim). + +- Use f-strings as long as the readability isn't impacted. + For more complex string formatting, use the shorter `%` syntax when features + special to `str.format()` aren't needed. + +- Otherwise, follow the + [PEP-8 standard](https://www.python.org/dev/peps/pep-0008/) + +### QML + +- Don't add trailing semicolons to lines + +- If an object has more than one property, always keep each property on their + own line: + + ```qml + Rectangle { x: 10; width: 100; height: width; color: "black" } // Bad! + + Rectangle { // Good + x: 10 + width: 100 + height: width + color: "black" + } + ``` + +- Don't use [States](https://doc.qt.io/qt-5/qml-qtquick-state.html). + The Rectangle in the description's example could simply be written like this: + + ```qml + Rectangle { + width: 100 + height: 100 + color: mouseArea.containsPress ? "red" : "black" + + MouseArea { + id: mouseArea + anchors.fill: parent + } + } + ``` + +- Otherwise, follow the + [QML Coding Conventions](https://doc.qt.io/qt-5/qml-codingconventions.html) + +### C++ + +- Add C++ only if it can't easily be done in QML or Python; + or if doing it in Python requires adding a dependency while a + similar feature is already provided by Qt, feature that just needs to be + exposed with some wrapper code + ([example](https://github.com/mirukana/mirage/blob/v0.6.4/src/utils.h#L31)). + +- Always use `this->` to refer to methods and class attributes, explicit is + better than implicit + +- Don't split modules between `.h` and `.cpp` files, this creates unnecessary + code repetition and is detrimental to performance when most functions will + contain very few lines and will only get included once before + starting the GUI. + + +## Resources + +Resources include background images, icons or sounds. +New resources must have a permissive license that does not require attribution. +Built-in icons must be in the SVG format. The majority of icons used in the +application come from [iconmonstr](https://iconmonstr.com). + +When possible without any noticable quality loss, reduce the size of +resources and strip any metadata by using tools such as: + +- `svgcleaner --allow-bigger-file --indent 4 ` for SVG images +- `pngquant --force --speed=1 --strip ` for PNG images +- `jpegoptim --quality 80 --strip-all ` for JPEG images + + +## Packaging + +If a new package for a distribution or any other easy way +of installing the application exists, [pull request](#pull-requests) for +adding instructions to the [INSTALL.md](INSTALL.md) are welcome. + +Some suggestions when creating packages: + +- As the `mirage` name is sometimes already taken by other software, + prefer naming your package `mirage-im` + +- Among the dependencies from the `submodules` directory, `hsluv-c` is the + only one that is still needed for building. + The other folders are kept to allow building past versions of the + application, and should be ignored.