-
Notifications
You must be signed in to change notification settings - Fork 115
Contribution Procedure
When you add code to the website (or any similar CompServ repo), you should follow a general procedure to make sure the contribution is orderly and reviewed.
Context: Around Fall 2019 and Spring 2020, the code contribution to hknweb was a mess. In particular, no one would review Pull Requests when they came in. Contributors assigned two to four reviewers, and none of them would review the pull request for weeks.
-
Create a fork of the Github repository, if you haven't done so yet.
-
Clone your forked repository locally to a computer, if you haven't done so yet. (If you haven't done these two steps, you can look at the Comprehensive Setup Guide for details steps on what to do)
-
Rebase from
compserv/master
to yourmaster
branch. Push the updated tip toorigin
. (There should be no conflicts, otherwise, you may have to merge and consult with other officers to determine what you changed and the next steps, like Pull Request from yourmaster
) -
Create a local branch for your project for your feature. Do all of your development in this branch, pushing to your forked
origin
repo as needed. -
Try to keep one kind of feature development in different Branches. If you need to mix features together, use your discretion or complete one Pull Request for one feature before creating another one.
-
Rebase or Merge from
compserv/master
if your branch is out of date, and fix any merge conflicts. -
On the Compserv repository, create a new Pull Request from your fork's branch to
compserv/master
. Title it with your fixes and/or creations. Link any related issues. -
Assign one to two officers and/or assistant officers in CompServ on GitHub using the "Assign" feature, or other committee members if applicable, to review the PR. If the PR is urgent, message them on Facebook or Slack, or use your discretion to merge in immediately. Continue pinging in a few days if needed.
-
Refer to Reviewing a Pull Request to start reviewing Pull Request
-
After checks finish running, review all errors and issues, and fix all issues.
-
If reviewers have any comments, address all comments by either fixing the issues or explaining why you think no changes are needed. Ask at least one reviewer to review your changes and comments. Repeat until approved.
-
Merge the pull request.
-
Every week, 1-3 people will be assigned to review Pull Requests for that week. Please let an officer know through Slack if you cannot review it within the week and we will assign someone else to do it in your place.
-
If the PR is urgent, review as soon as possible (use your judgement). Otherwise, review it within a week. A good time to go over PR reviews is after the weekly CompServ meetings. Look over the commits and files changed, and pull from the contributor's branch to test the code locally. Make comments asking for clarifications or justifications, and if warranted, request changes to be made or approve the pull request. Remember to publish your review.
-
When the contributor makes comments or changes, look over the changes. Make sure all of your points were satisfactorily addressed. Provide further comments or approve the PR.
(content from https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/checking-out-pull-requests-locally)
-
To test the functionality of the Pull Request, it is best to run the requester's code onto your computer. There are two ways to do this, with (i) being recommended and easiest:
- Clone the Requester's Repo and Get the Pull Request's Branch. You will first
git clone <REQUESTER_URL>
andgit checkout <BRANCH>
. You can see which branch is used on the top of the Pull Request: " wants to merge 8 commits into compserv:master from ". - To do this, navigate to the pull request, scroll down to the big button that says
Merge Pull Request
, and clickcommand line instructions
to the right of it. Follow these instructions to make a copy of the PR onto your computer.
- Clone the Requester's Repo and Get the Pull Request's Branch. You will first
-
Although you have the option to make edits and merge your changes, we typically do not do this. Instead, view
Files Changed
, where you can see the changes in the code itself and add your review directly referencing pieces of the code itself. Doing this greatly helps the requester understand exactly what in their code needs to be changed.- You can also make the changes yourself and send them to the reviewer via Slack to review so they can push it.
-
Additionally, for more general comments about the functionality of the code, you can go back to the
Conversations
tab and scroll all the way to the bottom of the page to leave a comment. These kinds of comments are warranted if something isn't working when you run the code, you have a general suggestion to make the PR better, you notice a frontend discrepancy with the rest of the website, etc.