-
Notifications
You must be signed in to change notification settings - Fork 59
Pull Request Guidelines
Once a PR has been reviewed, do not add more commits other than to resolve the comments. This makes it harder for reviewers and lengthens the time to get a PR through review.
A PR should ideally accomplish one thing. Good examples are:
- Decompiling a single larger function
- Decompiling multiple smaller functions
- Renaming a symbol
- Changing something about the build system
- Rearranging code
A PR that does all of those things is much harder to review.
This does not prohibit combining certain things that make logical sense. For example:
- Decompiling a function and renaming it can be in a single PR. However, if the renaming touches a lot of files it probably makes more sense to split it.
See https://github.com/Xeeynamo/sotn-decomp/blob/master/docs/NAMING.md and https://github.com/Xeeynamo/sotn-decomp/blob/master/docs/STYLE.md
All CI checks should be passing. This means a matching build for all versions and correct code formatting. See the build page for more information on checking this locally. https://github.com/Xeeynamo/sotn-decomp/wiki/Build
DECOMP_ME_WIP is part of the function_finder script that we use to tag decomp.me scratches for inclusion in the output. There are pros and cons to tagging a function:
Pros:
- If a function is renamed, the scratch will still be associated with it. Otherwise it would be lost.
- It can avoid function collisions with other projects or overlays since the matching is done by the function name.
- If the top scraped scratch is a questionable match, it can be overridden.
Cons:
- If there is a better scratch than the one in the source code, it won't be listed in the function_finder output.
The policy right now is that if someone goes to the trouble of submitting a PR requesting this, we will merge it. However it's not really necessary since the scraper should work in 99% of cases.
For entities, avoid using .ext.generic and .ext. Make make separate structs for every entity.