Process for submitting code changes

Jump to bottom

Jim Tyhurst edited this page Apr 2, 2017 · 7 revisions

Draft

Disclaimer

The purpose of this document is to set up a few conventions that will help everyone on the team work together better. Nothing here should be construed as exclusionary or trying to make distinctions between experts and newbies. We are just trying to set a few explicit guidelines, so that everyone can participate successfully, regardless of the amount of experience. We definitely do not want to create a bureaucracy and we do not want to create a bottleneck that will prevent good code from flowing into the repository quickly.

Definitions

Someone with privileges to commit in the repository is called a committer. We do not add any other restrictions. You do not need to be blessed by a technical lead. You do not need to know everything there is to know about the project or about the technologies used for the project. If you have been given write-rights for the repository, we refer to you as a committer and you can push branches to the repository and you can merge pull requests.

Someone who has not been given privileges to commit in the repository can still be a contributor and your code can appear in the project. However, you are not a committer, because you cannot push branches directly to the repository and you cannot merge pull requests. Our goal is to transform you from a contributor to a committer as soon as possible, but don't be offended if we ask you to provide a couple of pull requests as a contributor before we give you rights as a committer.

Process for submitting code changes

Nobody pushes changes directly to master branch. Instead, changes are merged from a pull request from a "feature branch" to the master branch.
Everybody, even committers, submits pull requests on a "feature branch" that is unique to the work represented in that pull request. For example, one branch for "Issue 42 - Sorting error", another branch for "Fixed typo in configuration file", etc.
Creating a pull request:

a. Committers can commit a "feature branch" directly to the hackoregon/team-budget repository and then create a pull request (PR).

b. Contributors can still create a pull request, but they need to fork the repository, create the "feature branch" on their fork, and then create a pull request. GitHub documentation provides a good explanation of this process.
After a PR has passed Travis tests it can be reviewed by committers. Go to the Files changed tab of the PR page and click the Review changes button:

a. If the code looks like an acceptable change, select the Approve option and add a comment.

b. If you believe improvements can be made to the code, select the Request changes option and explain what changes you believe are needed and why.

c. If you do not feel comfortable making a decisive judgement of the code, select the Comment option and submit your thoughts to enrich the conversation. Everyone is welcome to ask questions and make comments.
After one (1) committer approves the request, the pull request may be merged.

a. For a pull request from a committer: the original submitter usually does the merge and deletes the feature branch, because they proposed the change and they should know whether all of the other people's comments have been addressed.

b. For a pull request from a contributor: one of the committers needs to execute the merge and delete the feature branch, because the submitter does not have the repository rights to do that.

This review process leads to much better quality code and it is easier to fix/change/enhance on the branch than after it is merged. This process avoids everyone on the whole team having to deal with first draft code being merged too soon into the master branch.
In truly urgent cases, committers can do the merge without waiting for approval from anyone else. Please do not abuse this privilege.
After a contributor has had a couple of pull requests accepted, the team should strongly consider granting them committer privileges, so the list of committers keeps growing, usually very quickly.