Skip to content

Commit 757a5ca

Browse files
LpsdLpsd
committed
Initial commit
0 parents  commit 757a5ca

File tree

7 files changed

+914
-0
lines changed

7 files changed

+914
-0
lines changed

‎CODE_REVIEW.md

+125
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
### Contents:
2+
- [The most important thing](#the-most-important-thing)
3+
- [Context](#context)
4+
- [Everyone](#everyone)
5+
- [As a code submitter](#as-a-code-submitter)
6+
- [As a reviewer](#as-a-reviewer)
7+
- [Responding to feedback](#responding-to-feedback)
8+
9+
_These aren't necessarily "rules", it is just a guide on how to review code effectively._
10+
11+
It's been often said that programming is part art, part science - that because
12+
lots of times there's no single, simple solution to a problem; or if there is,
13+
we might not know about it. There's also an infamous joke that if there are *n*
14+
developers in the room, then there are *n+1* opinions on how things should be done.
15+
That being said, here are some guidelines that should prevent friction when submitting
16+
or reviewing code.
17+
18+
## The most important thing
19+
20+
**The code has to work.** Unless you open a PR as a work in progress, the code
21+
should be built and tested.
22+
23+
If you have changed build setup, it's useful to test the whole build from
24+
scratch (clean build). If you updated external libraries, test the pertaining
25+
features (e.g. if you updated the JSON library, test that all the Lua JSON functions
26+
work as expected). If you changed the BitStream version, test that your client/server
27+
still works with older/newer servers/clients.
28+
29+
Having people review your code is one thing, but you should not expect them to
30+
also *throughly test* the code for you.
31+
32+
### Context
33+
34+
One important thing that lots of guidelines forget to mention is the *context*
35+
of the pull request: sometimes it's a big refactor, sometimes it a new feature,
36+
sometimes it's a bug fix. Some of those might be more urgent than the others,
37+
and sometimes the code might not be perfect or extendable. **That's ok.**
38+
39+
## Everyone
40+
41+
* There is no perfect code: good enough is usually good enough. That being said,
42+
try to keep the number of WTFs per minute to a minimum. ![WTFs per minute](https://i.imgur.com/pFQNzHq.png)
43+
* Accept that many programming decisions are opinions.
44+
Discuss tradeoffs, which you prefer, and reach a resolution quickly.
45+
* Ask good questions; don't make demands ("What do you think about naming this :user_id?").
46+
* Good questions avoid judgment and avoid assumptions about the author's perspective.
47+
* Ask for clarification ("I didn't understand. Can you clarify?").
48+
* Avoid selective ownership of code. ("mine", "not mine", "yours")
49+
* Avoid using terms that could be seen as referring to personal traits ("dumb", "stupid").
50+
Assume everyone is intelligent and well-meaning.
51+
* Be explicit. Remember people don't always understand your intentions online.
52+
* Be humble. ("I'm not sure - let's look it up.")
53+
* Don't use hyperbole ("always", "never", "endlessly", "nothing"). Don't use sarcasm.
54+
* Keep it real. If emoji, animated gifs, or humor aren't you, don't force them.
55+
If they are, use them with aplomb.
56+
* Consider talking synchronously (e.g. chat) if there are too many
57+
"I didn't understand" or "Alternative solution:" comments.
58+
Post a follow-up comment summarizing the discussion.
59+
60+
When it boils down to it, remember that you're both on the same side—the goal is to make the code better.
61+
62+
## As a code submitter
63+
64+
In general:
65+
66+
* Try to keep the PRs small. There has been some research to indicate that beyond 400 LOC the ability to detect defects diminishes. (We're talking about LOC proper, vendor code and GUI layouts don't count.)
67+
* If the PR is work in progress, use GitHub's draft pull request feature.
68+
* Make a checklist on what's missing or could be improved in the PR description or as comments.
69+
70+
Your code:
71+
72+
* Explain why the code exists. ("It's like that because of these reasons. Would
73+
it be more clear if I rename this class/file/method/variable?")
74+
* Extract some changes and refactorings into future issues or pull requests.
75+
76+
Interacting with reviewers:
77+
78+
* Be grateful for the reviewer's suggestions. ("Good call. I'll make that change.")
79+
* A common axiom is "Don't take it personally. The review is of the code, not you." — this means you should be aware of how hard it is to convey emotion online and how easy it is to misinterpret feedback. If a review seems aggressive or angry or otherwise personal, consider if it is intended to be read that way and ask the person for clarification of intent, in person if possible.
80+
* Keeping the previous point in mind: assume the best intention from the reviewer's comments.
81+
* Seek to understand the reviewer's perspective.
82+
* Try to respond to every comment.
83+
* If you are waiting for a review or a response, it's not a problem to ping the #development channel.
84+
85+
If you have merge permissions:
86+
87+
* Wait to merge the branch until continuous integration tells you that the build has passed.
88+
* Merge once you feel confident in the code and its impact on the project.
89+
* Final editorial control rests with you.
90+
91+
## As a reviewer
92+
93+
Understand why the change is necessary (fixes a bug, improves the user
94+
experience, refactors the existing code). Then:
95+
96+
* Seek to understand the author's perspective.
97+
* Communicate which ideas you feel strongly about and those you don't.
98+
* Identify ways to simplify the code while still solving the problem.
99+
* Offer alternative implementations, but assume the author already considered
100+
them. ("What do you think about using a custom validator here?")
101+
* Remember that you are here to provide feedback, not to be a gatekeeper.
102+
* Ask, don’t tell. (“What do you think about trying…?” rather than “Don’t do…”)
103+
* Offer ways to simplify or improve code.
104+
* Communicate which ideas you feel strongly about and those you don't. Explain your reasons why code should be changed. (Not in line with the style guide? A personal preference?)
105+
* If you disagree strongly, consider giving it a few minutes before responding; think before you react.
106+
* Offer alternative implementations, but assume the author already considered them. ("What do you think about using a custom validator here?")
107+
* If discussions turn too theoretical or touch big architectural questions, move the discussion offline. In the meantime, let the author make the final decision on alternative implementations.
108+
* Don't keep the code hostage. Keep in mind the context and the most important thing - does it work?
109+
* Sign off on the pull request with a :thumbsup: or "Ready to merge" comment.
110+
111+
If the pull request author has merge permissions prefer review approvals instead of merging the PR for them.
112+
113+
## Responding to feedback
114+
115+
* Try to respond to every comment. Be grateful for the reviewer's suggestions. ("Good call. I'll make that change.")
116+
* Be aware of how hard it is to convey emotion online and how easy it is to misinterpret feedback. If a review seems aggressive or angry or otherwise personal, consider if it is intended to be read that way and ask the person for clarification of intent, in person if possible.
117+
* Link to any follow up commits or Pull Requests. (“Good call! Done in 1682851”)
118+
119+
If you have merge permissions:
120+
121+
* Wait to merge the branch until Continuous Integration tells you the build has passed. Merge once you feel confident in the code and its impact on the project.
122+
123+
----
124+
125+
This review guide is based on [work from thoughtbot inc](https://github.com/thoughtbot/guides/tree/master/code-review) and [work from @mrsasha](https://gist.github.com/mrsasha/8d511770ad9b282f3a5d0f5c8acdd10e).

‎CONTRIBUTING.md

+71
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
### Contents:
2+
- [What is mtasa-docs?](#what-is-mtasa-docs)
3+
- [Contributing to mtasa-docs](#contributing-to-mtasa-docs)
4+
- [Where to contribute](#where-to-contribute)
5+
- [Project structure](#project-structure)
6+
- [What to contribute](#what-to-contribute)
7+
- [Existing guidelines/documentation/practices](#existing-guidelinesdocumentationpractices)
8+
- [Discussions](#discussions)
9+
- [Who can contribute](#who-can-contribute)
10+
11+
# What is mtasa-docs?
12+
13+
The main goal of this repository is to provide a place for collaboration on documentation relating
14+
to multitheftauto projects (such as mtasa-blue, mtasa-resources, etc).
15+
16+
As a member of this community, you provide the time and effort (through your contributions) which helps
17+
keep our projects alive, and therefore should have a right to involvement in discussions around
18+
contribution guidelines and practices.
19+
20+
# Contributing to mtasa-docs
21+
22+
A lot of the ideas in our [Code review](CODE_REVIEW.md) document are
23+
applicable here.
24+
25+
## Where to contribute
26+
27+
From the root of this repository, each multitheftauto project lives within its own folder.
28+
For example `mtasa-blue` or `mtasa-resources`.
29+
30+
### Project structure
31+
General contribution information should be kept in `CONTRIBUTING.md`; where/what/how contributions should be made,
32+
as well as information on best practices.
33+
34+
Coding guidelines (exploring real code snippets and scenarios) should be kept in `CODING_GUIDELINES.md`
35+
36+
If a document requires a large snippet of code, it's best to store the code snippet separately under `examples/<example_name>`
37+
38+
For example (pun not intended), if writing the section on "Securing element data" for the [Script security](#) guide, the script example would be under `mtasa-blue/examples/securing_element_data.lua` and you'd link to that file in the guide.
39+
40+
## What to contribute
41+
42+
### Existing guidelines/documentation/practices
43+
If you're sure that something isn't quite right with any of our documentation, or is out of date by modern standards
44+
feel free to make a pull request addressing these changes. It's always worth having a quick search beforehand, through
45+
existing and previous issues/discussions, to ensure it hasn't already been discussed and decided on (at least, recently).
46+
47+
### Discussions
48+
Discussions may be created for topics which require some extra thought:
49+
https://github.com/multitheftauto/mtasa-docs/discussions
50+
51+
Please only contribute to discussions if what you are submitting adds value to the conversation.
52+
53+
- Have a solid point: "I just prefer it" isn't always going to win the argument.
54+
- Don't repeat previous points
55+
- Keep it friendly and encouraging for others to join in the discussion, see [Code review](CODE_REVIEW.md)
56+
57+
## Who can contribute
58+
59+
Anyone can contribute to the improvement of our documentation. We ask that you read this entire document well before
60+
doing so, as well as our [Code review](CODE_REVIEW.md) document. Please note, any kind of uncivilized behaviour may lead to restriction from the mtasa-docs repository.
61+
62+
- **Contributors** (regular users) are able to start discussions, open pull requests and issues.
63+
- **Members** (those with write access) are able to submit reviews for pull requests & merge them.
64+
- **Admins** (overlords) are able to manage everything, and have final say on any proposals that are 'stuck' due to lack of agreement
65+
66+
Pull requests must be reviewed by at least one repository member before they can be merged.
67+
68+
There _will be_ certain topics or discussions which are opinionated/preference-based, and we would like to avoid
69+
such topics going stale due to lack of agreement.
70+
71+
In this case, an admin of the repository is able to make a final decision on any proposals.

‎README.md

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# mtasa-docs
2+
This repository is for collaboration and storage of documentation relating to [mtasa-blue](https://github.com/multitheftauto/mtasa-blue) and [mtasa-resources](https://github.com/multitheftauto/mtasa-resources).
3+
4+
## Contribution
5+
It's recommended to start with the `CONTRIBUTING.md` for the respective project, explorting how your contributions should be made, as well as information on best practices:
6+
- [mtasa-blue](mtasa-blue/CONTRIBUTING.md)
7+
- [mtasa-resources](mtasa-resources/CONTRIBUTING.md)
8+
9+
10+
## Coding guidelines
11+
Or you can go directly to the `CODING_GUIDELINES.md`, exploring real code snippets and scenarios, useful for when you're contributing code and need to know our preferred conventions:
12+
- [mtasa-blue](mtasa-blue/CODING_GUIDELINES.md)
13+
- [mtasa-resources](mtasa-resources/CODING_GUIDELINES.md)
14+
15+
16+
## Code reviews
17+
Not looking to contribute with lines of code, but instead with your code review skills? See our [Code review](CODE_REVIEW.md) document, applicable to all of our projects, which may guide and inspire you to submitting great code reviews!

0 commit comments

Comments
 (0)