Difference between revisions of "Git Guidelines"

Latest revision as of 17:45, 23 October 2024

This page is mostly directed to core team members with commit or triage access to upstream repositories.

For instructions on basic usage, see Git.

For determining who is allowed to do what, see Organisation.

For guidelines about overall pull request quality, see Merging core pull requests to upstream.

Rules

Upstream branch rules

Feature freezes take effect in the master branch, and while the freeze is active, a separate development branch ("dev" or some other suitable name) is made into which pull requests are merged. It is then rebased onto master once the freeze is over. However, creating the branch may be skipped (it usually is!) when nobody feels like merging features during a feature freeze.

The minetest and minetest_game repositories contain the stable-0.4 branch, which has to be updated to the latest stable 0.4 series version at each release.

Upstream pull requests

Don't mix multiple things in one commit. The same applies to codestyle cleanup.
People considering merging pull requests are not required to look anything up anywhere else than the pull request and its comments. If there is something blocking the merging of a pull request, the reason must be added as a comment to the pull request. This goes both ways: If you check a pull request to be mergeable, write a simple "+1" comment to it.

Upstream commit rules

You can push something to upstream [1] only if two members of the core team [2] agree on it. (See also Organisation)
- Two for-votes are required for code to be mergeable upstream. Any against vote has to be resolved in a meeting before merge.
- For PRs: The second reviewer should invalidate the first review when major changes happened to the PR in the meantime.
Commit messages must start with a capital letter and must be in the present tense. (look at the commit log)
Do not modify history (i.e. force push) older than 10 minutes.
Use rebase, not merge, to get linear history. [3]
Do not rush with anything, unless our users' data is about to be corrupted otherwise.

Rule 1 in practice

Tell people openly what you do, and if someone finds a problem in what you do, allow resolving to take its time.

If you have a small patch, fixing some compiler error or other trivial mistake, notify about fixing it on #minetest-dev, wait for 5...15 minutes and push it. To save time, you should notify when finding the problem, not when having it fixed. If someone asks something about it, delay pushing and link the patch [4] or tell whatever else people want to know.

Rule 1 is only applied to the minetest/minetest and minetest/minetest_game repositories. For the other repos apply some common sense: Check who last worked on it or who wrote most of the code (if applicable), consider consulting them for changes especially if they're large. If nobody has cared about a repo for a long time you don't have to worry either.

Notes

[1] Upstream is at https://github.com/minetest/minetest

[2] The team: https://github.com/orgs/minetest/people

[3] On Github, press the "Rebase and merge" button. Of course you can rebase a remote branch in a local repository for more in-depth tools. There's also the ancient workflow of appending .patch to the pull request URL, getting into your project directory and doing git am <patch>. Similarly for single commits.

[4] Patches can be linked using a pastebin or by using GitHub (pull request or not).

Issue and Pull-Request Management

If an issue is a duplicate, post "duplicate of #ISSUENUM", label as Duplicate, and close the issue. Newer issues should be considered duplicates of older issues, unless the newer issue has more useful conversation. Information from the duplicate issue can also be edited into the open issue.
If a pull request or an issue does not get a response from its author within one month when requiring more details, it is closed.
- Use "Action / change needed" and "User feedback needed"
Core devs who reviewed a PR once should stay with the PR for additional review rounds. Loss of interest (thus unsubscribing) should be signalled properly.
- This is best communicated by assigning yourself to the PR using the GitHub feature.
- PR assignments show who's taking care of the PR; leaving the option to @ them to progress.
WIP / draft pull-requests that are not updated within 6 months should be closed.
Use Project Boards to prioritise and order issues and pull requests.
The Possible Close label can be used to warn authors of impending closure.

Triagers

Triagers are members of the project that are not core developers, but have the ability to manage issues - see above.
- Examples include labelling issues, asking for necessary information, and managing boards to help with long-term planning.
They may close issues or PRs, but cannot approve them (the act of approving an issue implies there is a dev willing to review a contribution).
They should err on the side of caution - if they don't understand the issue, they should wait for feedback.
They should consider ways to improve project management further.

@@ Line 1: / Line 1: @@
+This page is mostly directed to core team members with commit or triage access to upstream repositories.
 For instructions on basic usage, see [[Git]].
-== Upstream commit rules ==
+For determining who is allowed to do what, see [[Organisation]].
+For guidelines about overall pull request quality, see [[Merging core pull requests to upstream]].
+== Rules ==
+=== Upstream branch rules ===
-# You can push something to upstream [1] only if some other member of the core development team agrees on it [2]
+Feature freezes take effect in the master branch, and while the freeze is active, a separate development branch ("dev" or some other suitable name) is made into which pull requests are merged. It is then rebased onto master once the freeze is over. However, creating the branch may be skipped ('''it usually is!''') when nobody feels like merging features during a feature freeze.
-# Commit messages must look good and be in the present tense (look at the commit log)
-# Do not modify history older than 10 minutes
-# Use rebase, not merge, to get linear history. [3]
-=== Notes ===
+The <code>minetest</code> and <code>minetest_game</code> repositories contain the stable-0.4 branch, which has to be updated to the latest stable 0.4 series version at each release.
-[1] Upstream is at https://github.com/minetest
+=== Upstream pull requests ===
-[2] The team: https://github.com/minetest?tab=members
+* Don't mix multiple things in one commit. The same applies to codestyle cleanup.
+* People considering merging pull requests are not required to look anything up anywhere else than the pull request and its comments. If there is something blocking the merging of a pull request, the reason must be added as a comment to the pull request. This goes both ways: If you check a pull request to be mergeable, write a simple "+1" comment to it.
-[3] For a github pull request, this is easiest done by appending .patch to the pull request URL, wgetting it to your project directory and doing <code>git am whatever.patch</code>. Similarly for single commits.
+=== Upstream commit rules ===
+* You can push something to upstream [1] only if two members of the core team [2] agree on it. (See also [[Organisation]])
+** Two for-votes are required for code to be mergeable upstream. Any against vote has to be resolved in a meeting before merge.
+** For PRs: The second reviewer should invalidate the first review when major changes happened to the PR in the meantime.
+* Commit messages must start with a capital letter and must be in the present tense. (look at the commit log)
+* Do not modify history (i.e. force push) older than 10 minutes.
+* Use rebase, not merge, to get linear history. [3]
+* Do not rush with anything, unless our users' data is about to be corrupted otherwise.
 == Rule 1 in practice ==
-Tell people openly what you do, and if someone finds a problem in what you do, allow resolving to take it's time. Communicate both, the big goals and the important details beforehand to lessen wasted work.
+Tell people openly what you do, and if someone finds a problem in what you do, allow resolving to take its time.
+If you have a '''small patch''', fixing some compiler error or other trivial mistake, notify about fixing it on #minetest-dev, wait for 5...15 minutes and push it. To save time, you should notify when ''finding'' the problem, not when ''having it fixed''. If someone asks something about it, delay pushing and link the patch [4] or tell whatever else people want to know.
+Rule 1 is '''only''' applied to the <code>minetest/minetest</code> and <code>minetest/minetest_game</code> repositories.
+For the other repos apply some common sense: Check who last worked on it or who wrote most of the code (if applicable), consider consulting them for changes especially if they're large. If nobody has cared about a repo for a long time you don't have to worry either.
+==== Notes ====
-If you have a '''small patch''', fixing some compiler error or other trivial mistake, notify about fixing it on #minetest-dev, wait for 5...15 minutes and push it. To save time, you should notify when ''finding'' the problem, not when ''having it fixed''. If someone asks something about it, delay pushing and link the patch [1] or tell whatever else people want to know.
+[1] Upstream is at https://github.com/minetest/minetest
-If you have a '''medium-sized patch''', usually containing 5...50 changed lines, you should have had already notified people about what you started doing. Link the patch [1] on #minetest-dev and wait until people notice it. Link again later or the next day or next week if needed. [2] Merge after more people agree than disagree on it, and prepare to rework it more or less if needed. Aim for a consensus.
+[2] The team: https://github.com/orgs/minetest/people
-If you have a '''large patch''', containing >100 changed lines, you should have notified on #minetest-dev when you started coding it, telling what problem or missing feature you are addressing and/or noting midway what specific approach you took. When notifying the first time, try to make sure [2] someone knowledgeable approves your goal and doesn't disagree with your approach. Continue like with a medium-sized patch.
+[3] On Github, press the "Rebase and merge" button. Of course you can rebase a remote branch in a local repository for more in-depth tools. There's also the ancient workflow of appending .patch to the pull request URL, getting into your project directory and doing <code>git am <patch></code>. Similarly for single commits.
-It is always polite to give more time for people to look at your patch, especially if not many people are present. It also lets you take a break and think, and generally nothing is lost. If you fear people judging you, you have ''even more'' reason to communicate.
+[4] Patches can be linked using a pastebin or by using GitHub (pull request or not).
-Some people tend to overly and negatively react to certain kinds of things. In these cases it's best to just let the steam first go out and then move to a less escalated and more factful discussion, forgetting the initial reaction.
+== Issue and Pull-Request Management ==
-This text only refers to patches that you want to get to upstream soon. You are encouraged to code proof-of-concept patches of all sizes for discussion and testing without intention to push them upstream.
+* If an issue is a duplicate, post "duplicate of #ISSUENUM", label as [https://github.com/minetest/minetest/labels/Duplicate Duplicate], and close the issue. Newer issues should be considered duplicates of older issues, unless the newer issue has more useful conversation. Information from the duplicate issue can also be edited into the open issue.
+* If a pull request or an issue does not get a response from its author within one month when requiring more details, it is closed.
+** Use [https://github.com/minetest/minetest/labels/Action%20%2F%20change%20needed "Action / change needed"] and [https://github.com/minetest/minetest/labels/User%20feedback%20needed "User feedback needed"]
+* Core devs who reviewed a PR once should stay with the PR for additional review rounds. Loss of interest (thus unsubscribing) should be signalled properly.
+** This is best communicated by assigning yourself to the PR using the GitHub feature.
+** PR assignments show who's taking care of the PR; leaving the option to @ them to progress.
+* [https://github.com/minetest/minetest/labels/WIP WIP] / draft pull-requests that are not updated within 6 months should be closed.
+* Use [https://github.com/minetest/minetest/projects Project Boards] to prioritise and order issues and pull requests.
+* The [https://github.com/minetest/minetest/labels/Possible%20Close Possible Close] label can be used to warn authors of impending closure.
-=== Notes ===
+=== Triagers ===
-[1] Patches can be linked using a pastebin or by using github (pull request or not).
+* Triagers are members of the project that are not core developers, but have the ability to manage issues - see above.
+** Examples include labelling issues, asking for necessary information, and managing boards to help with long-term planning.
+* They may close issues or PRs, but cannot approve them (the act of approving an issue implies there is a dev willing to review a contribution).
+* They should err on the side of caution - if they don't understand the issue, they should wait for feedback.
+* They should consider ways to improve project management further.
-[2] It's really some kind of an interestocracy. If nobody is interested of something, you should wait for a silent moment and then eg. explain what it does; how much the current version sucks and how little it sucks after the patch; what kind of performance measurements you made; tell people what to try to do to easily see a problem. Try telling the big picture or a detail. --[[User:Celeron55|Celeron55]] ([[User talk:Celeron55|talk]]) 01:14, 5 August 2013 (MSK)
+[[Category:Rules and Guidelines]]
+[[Category:Git]]