Skip to content
The Holocron

Pull Requests

If you’re unfamiliar with the Github flow, please read this guide as an introduction. Make sure you reach out to someone on the team before you start any work to ensure that what you’re doing is aligned with the teams’ vision. Read Ilya Grigorik’s great article Don’t “Push” Your Pull Requests. The following passage sums up some great points:

Contributions are always welcome, but surprise patches are mostly a burden. Yes, you are offering your help, but someone else will have to maintain your code over the long term - get their buy-in first, avoid surprises. Even worse, a localized change to address a specific problem will often miss the full implications to the project: other existing use cases, future roadmap plans, or overall architectural decisions. A good idea can be implemented inappropriately for the specific project; it can be invalidated by another effort that you may not even be aware of; the timing may be wrong, and a dozen other reasons can conspire against you.

  1. Clone the repo
  2. If you’ve cloned previously, then get latest changes (git checkout main; git pull origin main).
  3. Create your feature branch (git checkout -b <my-new-feature>); checkout the guide below for our branching strategy.
  4. Commit your changes (git commit), following the format suggested below. Please don’t use the shortcut flag git commit -m <message> as there is more detail required than what is allowed by only using that flag.
  5. Push to the branch (git push origin <my-new-feature>)
  6. Create a new Pull Request; make sure you fill in the Pull Request template.

Filling Out a Feature Request

Now its time to submit a ticket. Feature requests should be thorough and not leave anyone wondering or questioning what you were thinking. No one should have to ask you anything or require further clarification. When creating a ticket, use the Feature Request template to populate a lot of the information required. Below is a brief description of the parts of a feature request:

  • Is your feature request related to a problem?: A clear and concise description of what the problem is. Ex. I’m always frustrated when […]
  • Describe the solution you’d like: A clear and concise description of what you want to happen. Acceptance criteria are important for us to ensure a proper and complete feature.
  • Describe alternatives you’ve considered: A clear and concise description of any alternative solutions or features you’ve considered.
  • Additional context: Add any other context or screenshots about the feature request here.

Example Feature Request

Follow the example below and you’ll be on the right path.

Title: Progress bar component


**Is your feature request related to a problem? Please describe.**
Not related to a problem. I would like to see an indicator of impressions to ad requests
so that I can visualize the loss.


**Describe the solution you'd like**
I think a progress bar where the total value shown is the full width of
the contained area, and the part that is filled in is the impressions.


**Describe alternatives you've considered**
A funnel chart was considered but its too similar to Googles design and
Legal fears repercussions.


**Additional context**
N/A

Gotchas

  • Please avoid working directly on the main branch.
  • Make commits of logical units.
  • Check for unnecessary space with git diff --check before committing.
  • Make sure your commit messages are in the proper format, also listed below in our guides.

Review Pull Requests

Permission Required: Contributor

While anyone can comment on a PR, add feedback, etc, PRs are only approved by team members with Contributor or higher permissions.

PR reviews use GitHub’s own review feature, which manages comments, approval, and review iteration.

ALL PULL REQUESTS being approved must:

  • be approved by at least 2 reviewers, one of which should come from a code owner from within the CODEOWNERS file, unless submitted by a Maintainer or above.
  • be up-to-date and have no conflicts with the main branch.
  • be covered by a test either by a previously-failing test, an existing test that covers the entire functionality of the submitted code, or new tests that verify any new/changed behavior.
  • have all tests pass and follow established conventions.
  • not drop the test coverage percentages, unless the specific case is considered reasonable by maintainers.
  • pass our linting suite 100% with no errors on both the JavaScript and SCSS side

Some notes about reviewing:

  • You may ask for minor changes (“nitpicks”), but consider whether they are really blockers to merging: try to err on the side of “approve, with comments”.
  • Please make sure you’re familiar with the code or documentation being updated, unless it’s a minor change (spellchecking, minor formatting, etc). You may @mention another project member who you think is better suited for the review, but still provide a non-approving review of your own. Check the CODEOWNERS file for a list of people you can mention when in doubt.
  • Be extra kind: people who submit code/doc contributions are putting themselves in a pretty vulnerable position, and have put time and care into what they’ve done — always respond with respect, be understanding, but don’t feel like you need to sacrifice your standards for their sake, either; please don’t be a jerk about it.

Merge Pull Requests

Permission Required: Maintainer

After a pull request has been given the green light, it’s time to prepare for the deployment.

Preparation

  1. Merge your feature branch into main by pressing the button on GitHub’s interface. Use the Squash and merge button in case there are other options. It’s okay to merge multiple features and release once for those different features.
  2. Ensure that the title of the PR is in the form of conventional commits. If you named the title of your PR in this format then it should follow through when you go to merge.
  3. Read below for versioning.