-
-
Save celeron55/08457aacf814776a9363 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merging pull requests into Minetest Core | |
---------------------------------------- | |
As seen in practice, there are five major requirements that each pull request | |
must fullfill in order to be mergeable to upstream Minetest. There exists no | |
valid reason for anything to be merged without all of these passing. | |
1) It must follow a roadmap in some way, to make sure it fits the whole picture | |
of the project. The most up-to-date, reliable, feasible and/or authoritative | |
roadmap available today is probably celeron55's. [1] | |
2) It must work in the first place. Compile it and test it in game, or write mod | |
code that uses it. | |
3) The code style must be correct. [2] | |
4) The internal interfaces of the code must be good, and it should be reasonably | |
optimized, depending on how often the code is called. | |
5) 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. | |
These can be checked by: | |
1) Anyone. | |
2) Anyone, or at least any modder. | |
3) Those with a bit of general C++ and Lua knowledge. | |
4) Those with a lot of general C++ and Lua knowledge. | |
5) Those who have studied the file formats, the protocol and the Lua API, and | |
know how version compatibility can be reliably done. | |
I (celeron55) am interested in seeing whether we could end up in a situation | |
where pull requests would get comments like "req123 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. | |
[1] https://forum.minetest.net/viewtopic.php?f=7&t=9177&p=139455 | |
[2] http://dev.minetest.net/Code_style_guidelines |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment