Update CONTRIBUTING.md
This commit is contained in:
Risto Lahtela
GitHub
parent
d127030616
commit
c25fa693c6
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue