-
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 your origin's
master
withcompserv/master
if there are changes since you started your project Example git commands:git remote add upstream <COMPSERV-REPO-URL> # Do this line if you haven't yet added a CompServ repo upstream (with the ".git" in the end) git fetch upstream # Fetch from "upstream", or CompServ repo git checkout master # go to your local "origin" master branch git rebase upstream/master
It is HIGHLY recommended you
rebase
rather thanmerge
your fork's master. If you have to merge or make changes to your rebase, that means you made changes to yourmaster
branch.If you have to make changes after
rebase
, attempt to fix the merges. If you need to, you can then considermerge
bygit merge upstream/master
. Then make a new branch to keep the changes, having that branch point to whatmaster
is at this point, and then usegit branch -f master <COMPSERV-MASTER-COMMIT-ID>
andgit push -f
to 1) transfer your changes and edits onto another branch, and 2) keep your master up to date and consistent withcompserv/master
. -
Rebase or Merge your origin's
master
to your project branch if your branch is out of date, and fix any merge conflicts. Example git commands are:git checkout <BRANCH> # go to your BRANCH git rebase master # rebase / merge your current branch (HEAD on BRANCH) with master # OR git merge master
-
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:"<REQUESTER> wants to merge 8 commits into compserv:master from <BRANCH>"
. - 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.