Skip to content

Contribution Procedure

Jump to bottom
bri25yu edited this page Nov 29, 2022 · 15 revisions

Before starting development

  1. Create a fork of the Github repository, if you haven't done so yet.
  2. Clone your forked repository locally to a computer git clone https://github.com/<your_username>/hknweb
  3. Rename your fork git remote rename origin <fork_name>
  4. Add compserv as a remote git remote add compserv https://github.com/compserv/hknweb
  5. Pull from compserv/master into your <fork_name>/master.
  6. Create a local branch for your project for your feature git checkout -b <branch_name>.
  7. Push your branch to Github git push --set-upstream <fork_name> <branch_name>.

After development

  1. Update your branch from compserv/master i.e. git pull compserv master and resolve any merge conflicts.
  2. 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.
  3. Assign one to two officers and/or assistant officers in CompServ on GitHub using the "Assign" feature to review the PR.
  4. After checks finish running, review all errors and issues, and fix all issues.
  5. If reviewers have any comments, address all comments by either fixing the issues or explaining why you think no changes are needed. Repeat until approved.
  6. Merge the pull request :D woohoo!

Details Regarding Reviewing a Pull Request

(content from https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/checking-out-pull-requests-locally)

  1. 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:

    1. Clone the Requester's Repo and Get the Pull Request's Branch. You will first git clone <REQUESTER_URL> and git 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>".
    2. To do this, navigate to the pull request, scroll down to the big button that says Merge Pull Request, and click command line instructions to the right of it. Follow these instructions to make a copy of the PR onto your computer.

  2. 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.

  3. 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.

Best practices

Critique the code, not the contributor.

High-quality code is good. Long-term contributions are better.

HKN, by its nature, has a high turnover rate, and compserv has always been small. Encouraging long-term contributions should be our first priority, over high-quality code. Allow good, but flawed, code to be merged, and teach people to write better code.

Code can be fixed later, but only if someone is around to fix it.

Don't:

  • Insult code
  • Insult people
  • Rejection without justification

Do:

  • Offer alternative implementations / strategies
  • Link to specific documentation
  • Phrase comments as suggestions, with justifications
  • Assume basic competency (we are students at Berkeley)
  • Act in good faith (unless otherwise proven beyond a reasonable doubt)

Technical standards

At every level we should strive for the following (among others I've forgotten):

  • Readability: can future maintainers (or you in the near future) understand this? Or will they miss crucial context?
  • Efficiency: is this time / space efficient (algorithmic, db I/Os, latency)?
  • Usability: are different use-cases (admins, officers, candidates, students, faculty) easy to use?
  • Accessibility: can all users use our site? (vision-impaired, motor-impaired, limited bandwidth, etc.)
  • Security: can our data be compromised? Are we vulnerable to DoS?
  • Aesthetics: will people enjoy using our site? Does our UI support usability?
Clone this wiki locally