Game-Decompilation/2ship2harkinian
1
0
Fork 0
mirror of https://github.com/HarbourMasters/2ship2harkinian.git synced 2025-02-25 08:42:47 +00:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

Adds a REVIEWING.md documentation file (#160)

Browse Source 
* Adds a first pass of REVIEWING.md

* Update REVIEWING.md

* Clarifies better what files can be in the header.

* More clarity on how to resolve jenkins build errors.

* Adds a link to the Code Review Guidelines to README.md

* Adds code review guidelines link to contributing.md

* Clarifies trello procedures.

* Adds warning that NON_MATCHING equivalence isn't reviewed for yet.
This commit is contained in:
Kenix3 2021-05-30 11:20:29 -04:00 committed by GitHub
parent 2f239c939d
commit f85feadc27
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 72 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
CONTRIBUTING.md
View File

@ -17,6 +17,8 @@ Useful Links
- [Trello board](https://trello.com/b/ruxw9n6m/majoras-mask-decompilation) - We use this to track decompilation progress, not GitHub Issues.
- [Installation guide](https://github.com/zeldaret/mm/blob/master/README.md#installation) - Instructions for getting this repository set up and built on your machine.
- [Code Review Guidelines](REVIEWING.md) - These are the guidelines that reviewers will be using when reviewing your code. Good to be familiar with these before submitting your code.
- [Zelda 64 Reverse Engineering Website](https://zelda64.dev/) - Our homepage, with links to other resources.
- [Introduction to OOT decomp](https://github.com/zeldaret/oot/blob/master/docs/tutorial/contents.md) - A very detailed tutorial on how to get started with decomp. For OOT, but largely applicable to MM as well.
- The `#resources` channel on the Discord contains many more links
@ -49,7 +51,7 @@ An *actor* is any thing in the game that moves or performs actions or interactio
You should [join the Discord](https://discord.zelda64.dev/) to say hello and get suggestions on where to start on the `#mm-decomp` channel.
We track who is working on what on the [Trello board](https://trello.com/b/ruxw9n6m/majoras-mask-decompilation).
After joining the Discord, ask to be added to the Trello board so you can claim your actor and avoid duplicate work.
After joining the Discord, ask to be added to the Trello board so you can claim your code file and avoid duplicate work. You may need to add a card if a card for your file does not currently exist.
Style Guide & Conventions
-------------------------
@ -144,7 +146,7 @@ Feel free to reach out on the Discord if you have any questions about these step
### Pull Request Process
After opening a PR, the Jenkins server will verify your changes.
If there is an error, double-check that you can successfully `make clean && make` locally and that all added/modified files were `git add`-ed to your commit.
If there is an error, double-check that you can successfully `make clean && make` locally. If the build is OK, the next thing to check is that all added/modified files were `git add`-ed to your commit. The final check before posting on Discord for help is that there are no new warnings added to the code causing Jenkins to fail. You can check this by running: `tools/warnings_count/check_new_warnings.sh`.
Each PR needs a review from one reviewer and the project lead.

2
README.md
View File

@ -125,4 +125,4 @@ Anyone who wishes to contribute to the OOT or MM projects **must not have access
Most discussions happen on our [Discord Server](https://discord.zelda64.dev), where you are welcome to ask if you need help getting started, or if you have any questions regarding this project and other decompilation projects.
For more information on getting started, see our [Contributing Guide](CONTRIBUTING.md).
For more information on getting started, see our [Contributing Guide](CONTRIBUTING.md) and our [Code Review Guidelines](REVIEWING.md) to see what code quality guidelines we follow.

67
REVIEWING.md Normal file
View File

@ -0,0 +1,67 @@
Reviewing Pull Requests to the Majora's Mask Decompilation Project
=======================================================
Thanks for helping us reverse engineer *The Legend of Zelda: Majora's Mask* for the N64!
We invite all contributors to participate in our code review program. Every review submitted helps us keep code quality high and code merged in more quickly.
This document is meant to be a set of tips and guidelines for reviewers of pull requests to the project.
For general information about the project, see [our readme](https://github.com/zeldaret/mm/blob/master/README.md).
Most discussions happen on our [Discord Server](https://discord.zelda64.dev) where you are welcome to ask if you need help getting started, or if you have any questions regarding this project and other decompilation projects.
Useful Links
------------
- [Discord](https://discord.zelda64.dev/) - Primary discussion platform.
- [Trello board](https://trello.com/b/ruxw9n6m/majoras-mask-decompilation) - We use this to track decompilation progress, not GitHub Issues.
- [Contributing guide](https://github.com/zeldaret/mm/blob/master/CONTRIBUTING.md) - Instructions for contributors.
- [Installation guide](https://github.com/zeldaret/mm/blob/master/README.md#installation) - Instructions for getting this repository set up and built on your machine.
- [Zelda 64 Reverse Engineering Website](https://zelda64.dev/) - Our homepage, with links to other resources.
- [Introduction to OOT decomp](https://github.com/zeldaret/oot/blob/master/docs/tutorial/contents.md) - A very detailed tutorial on how to get started with decomp. For OOT, but largely applicable to MM as well.
- The `#resources` channel on the Discord contains many more links
Getting Started
---------------
### What should I know to take part in the review process?
You should first get yourself familiar with our [Contributing guide](https://github.com/zeldaret/mm/blob/master/CONTRIBUTING.md). It is also recommended that you have already successfully submitted a merged pull request to understand how the process works before submitting a review.
Pull Requests (PRs)
------------
### How to review a Pull Request
- When reviewing a PR it is suggested that you follow the checklist outlined in this section.
- You are not required to check for all of the items in the checklist. It is meant as a guide.
- Once the PR author has addressed all of your comments, you should add a comment with approval to the PR to signify to the project leads that this PR has been through peer review.
- If someone does not address your comments and expresses that a different way is better than yours, look for feedback from other contributors. The project lead will have final say in these situations. All decisions will generally be guided by a consensus of contributors.
### Reviewer Checklist
- [ ] Jenkins build is successful.
- [ ] `make` builds a matching ROM.
- [ ] Any new compiler warnings that were added are required for matching. Ensure there is good reason if the warnings files have changed. `./tools/warnings_count/warnings_build_current.txt` and `./tools/warnings_count/warnings_setup_current.txt`
- [ ] Files with NON_MATCHING functions have equivalent behaviour. (This will only be reviewed once we have shiftability since equivalence testing is easiest in game)
- [ ] `code` and `boot` segment files should be documented as much as possible. Overlays with documentation should be complete.
- [ ] Overlays with documentation should have macros to define access to parameters if the parameter uses bitwise access. The params should have an enum when it makes sense.
- [ ] `format.sh` was run.
- [ ] Comments and variables have correct spelling.
- [ ] The following should be declared in an Actor header file. There should be nothing else in the Actor header file.
- [ ] Main Actor struct
- [ ] Extern'd initVar data.
- [ ] Types used in the actor struct. Specific example would be actionFunc typedefs.
- [ ] Param field macros and/or enums.
- [ ] For any other additional `enum`/`struct`/`define`/`function`/`global`, there needs to be evidence it is needed in another file.
- [ ] New variables and functions should follow standard naming conventions.
- [ ] Constants are converted to whichever looks best in context: hexadecimal, decimal, or float
- [ ] Rotation angles should always be in hexadecimal
- [ ] Colour values should always be in decimal.
- [ ] Functions, structs, unions, enums, and typedefs are `TitleCase` (`DmRavine`)
- [ ] "Methods" for objects separate the system from the verb with an underscore (`DmRavine_Init`)
- [ ] Variable names are `camelCase` (`actionFunc`)
- [ ] Global variables start with `g` (`gSaveContext`)
- [ ] Static global variables start with `s` (`sSphereInit`)
- [ ] Macros and enum constants are `SCREAMING_SNAKE_CASE` (`DM_RAVINE_STATE_ACTIVE`)
- [ ] Trailing commas in array and struct definitions (see `EnJcMatoDamageTable`)