Difference between revisions of "Merging core pull requests to upstream"
Jump to navigation
Jump to search
| (2 intermediate revisions by 2 users not shown) | |||
| Line 4: | Line 4: | ||
For guidelines and rules on Git and Github, see [[Git Guidelines]]. | For guidelines and rules on Git and Github, see [[Git Guidelines]]. | ||
| + | |||
| + | Also see: | ||
| + | * https://github.com/minetest/minetest/blob/master/.github/CONTRIBUTING.md | ||
| + | * https://github.com/minetest/minetest/blob/master/doc/direction.md | ||
== Requirements == | == Requirements == | ||
| Line 9: | Line 13: | ||
There are five major requirements that each pull request must fullfill in order to be mergeable to upstream Minetest. | There are five major requirements that each pull request must fullfill in order to be mergeable to upstream Minetest. | ||
| − | # It | + | # It should follow a roadmap in some way, to make sure it fits the whole picture of the project. Different roadmaps and project guidance is managed in https://github.com/minetest/minetest/blob/master/doc/direction.md. A core dev can decide to review and merge something that doesn't follow direction.md if they consider it to be beneficial to the project. |
# It must work in the first place. Compile it and test it in game, or write mod code that uses it. | # It must work in the first place. Compile it and test it in game, or write mod code that uses it. | ||
# The code style must be correct. http://dev.minetest.net/Code_style_guidelines | # The code style must be correct. http://dev.minetest.net/Code_style_guidelines | ||
| Line 29: | Line 33: | ||
someone could do the rest and ultimately anyone could merge it based on these | someone could do the rest and ultimately anyone could merge it based on these | ||
simple comments, after each of the five requirements have been checked. | simple comments, after each of the five requirements have been checked. | ||
| + | [[Category:Git]] | ||
Latest revision as of 16:08, 24 September 2022
This page contains technical guidelines for core developers when deciding whether to merge a pull request.
For determining who is allowed to do what, see Organisation.
For guidelines and rules on Git and Github, see Git Guidelines.
Also see:
- https://github.com/minetest/minetest/blob/master/.github/CONTRIBUTING.md
- https://github.com/minetest/minetest/blob/master/doc/direction.md
Requirements
There are five major requirements that each pull request must fullfill in order to be mergeable to upstream Minetest.
- It should follow a roadmap in some way, to make sure it fits the whole picture of the project. Different roadmaps and project guidance is managed in https://github.com/minetest/minetest/blob/master/doc/direction.md. A core dev can decide to review and merge something that doesn't follow direction.md if they consider it to be beneficial to the project.
- It must work in the first place. Compile it and test it in game, or write mod code that uses it.
- The code style must be correct. http://dev.minetest.net/Code_style_guidelines
- The internal interfaces of the code must be good, and it should be reasonably optimized, depending on how often the code is called.
- The protocols and formats that it uses must be well designed, including the required compatibility in the part in question. On-disk formats are extremely important to get right. Modding API concerns are split between this and req2. This is about knowing the engine's design along with its pitfalls.
...which can be checked by:
- Anyone.
- Anyone, or at least any modder.
- Those with a bit of general C++ and Lua knowledge.
- Those with a lot of general C++ and Lua knowledge.
- Those who have studied the file formats, the protocol and the Lua API, and know how version compatibility can be reliably done.
How can this help?
I (celeron55) am interested in seeing whether we could end up in a situation where pull requests would get comments like "req 1, 2 and 3 checked.", after which someone could do the rest and ultimately anyone could merge it based on these simple comments, after each of the five requirements have been checked.