diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 18fccf644..0eb261b8f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,35 +1,85 @@ -# Writing a good issue report +# Writing a good issue In order to be useful for the maintainers issues should include as much information as possible about the issue. The information is crucial for reproducing the issue. -- Tickets about Exceptions should include a full stack trace if at all possible. (Usually found in the Errors.txt file or console logs) -- Tickets about Visual issue such as wrong characters or typos should include a screenshot of the affected window as well as what should be displayed instead. -- Tickets about odd behaviour should include a detailed turn of events how this issue occurred, if at all possible. +## :warning: Good exception report -Additional information that might be useful: +- [Example of a good issue report for an Exception](https://github.com/plan-player-analytics/Plan/issues/945) -- Possible logs, debug logs, error logs -- Plan and Server version & type -- Database in use (SQLite / MySQL) -- Possible config settings related to the issue -- Information related to the issue in the database -- Table structure in the database if related to the issue +### Contains + +- Description about what is wrong, or what was done before the issue happened. +- Version information, Server information and what database is in use +- Exception pastebin/gist link or inside code block (Surrounded by 3 ``` characters) + +## :eyeglasses: Good visual bug report + +### Contains + +- Description about what is wrong +- Full page screenshot containing the visual issue, with issue possibly highlighted +- Version information, are any custom html files being used + +## :coffee: Good feature request + +- [Example of a good feature request](https://github.com/plan-player-analytics/Plan/issues/872) + +### Contains + +- Description about what the feature solves (How can this be used?) +- Description about the feature +- Alternative features that would solve the same use case you have considered # Writing a good Pull Request -Good pull requests make the work of maintainers easier, which will make the approval of the pull requests quicker. -New code should be created in a new feature / bugfix / improvement branch. -After done with the feature (Or prior) a PR can be opened. +Development of new versions is done on `development`-branch, or sometimes shortlived feature branches. `master` contains latest release. +Keep this in mind when choosing where to start your work on. -Good practices that make PR easier to approve: +## :tophat: Good Pull Request -- Commit often with detailed commit messages: Commits are cheap and can be easily reversed if neccessary. -- Before marking an issue solved in a commit message ("Fixed #issueN.o.") it should be ensured that the issue is fixed. (By manual testing or with Unit Tests.) -- Name PR related to what it is attempting to accomplish. "Implemented improved graph creation", "Bugfix PR for Version 4.0.2" -- In case the feature in PR is not self explanatory an attempt to explain the feature should be made in the message / comments of the PR. -- Code follows similar style as rest of the code and is easy to read. (Brackets used always, Classes with BigFirstLetter, variablesCamelCase) +### Contains -IF you do not want your PR to be merged yet, include WIP in the title of the PR. +- 1 to 750 changes +- Descriptive commit messages +- Summary of what the PR contains +- Automatically or manually tested code + - (Summary of manual tests) -PRs are never merged directly to the `master`-branch, and are instead merged into next version specific branch. -IF no version specific branch is available when making a PR, select master and notify in the comments about the fact - Maintainers will create a new version specific branch and change the branch of the PR. +### Code + +You can code with whichever style you find most comfortable, as long as it is readable, but be aware that it will be formatted after your PR has been merged. + +- Variables describe what they contain +- Methods do one thing (Single Responsibility Principle) +- Comments + - When reason for doing something might be unclear, it is commented ("The value might be over 100, so it has to be checked") + - When something might break in the future, it is commented ("MySQL v5.6.2 might optimize this fix away") + - When something is unclear, it is commented ("I don't know if this works") +- Magic numbers are named with variables (`getItem(id)` instead of `API.getItem(5)`) + +### Testing + +Sometimes things do not work how they should, so testing is a good practice. + +- [Instructions for running build and test goals](https://github.com/plan-player-analytics/Plan/wiki/Project-Setup#building-and-testing) + +Because plugins often require mocking for tests, I do not require automatic tests for PRs. + +## :chart_with_upwards_trend: Good Review on a Pull Request + +Good reviews should leave both the reviewer and the contributor positive feeling about the interaction. + +### Contains + +- Questions about possible concerns about the code +- Notes about possible bugs or mistakes +- Positive encouragement + - (Advice) + +### Does not contain + +- Comments against the person submitting the PR +- Fast to fix things + - Formatting advice about small things ("If blocks should use brackets") + - Nitpicking ("This variable should be called bar instead of foo") + - Typos