Skip to content

Latest commit

 

History

History
211 lines (138 loc) · 7.93 KB

pull-requests.md

File metadata and controls

211 lines (138 loc) · 7.93 KB

Pull Requests

Screenshots

Include screenshots and animated GIFs in your pull request whenever possible.

Recording GIFs There are many great tools for recording a GIF. On a mac, we recommend recordit, which is a free lightweight app. If you are using Ubuntu, you can also try using Peek.

GIF Example

![](http://g.recordit.co/6dE0EmM29Z.gif)

Tables: When there are multiple screenshots, such as a style change that affects different themes or rtl, it can be nice to use a table for the screenshots docs

Table Example
Firebug Light
firebug light
|Firebug|Light|
|----------|------|
|![firebug](https://cloud.githubusercontent.com/assets/1755089/22209733/94970458-e1ad-11e6-83d4-8b082217b989.png)|![light](https://cloud.githubusercontent.com/assets/1755089/22209736/9b194f2a-e1ad-11e6-9de0-561dd529d5f0.png)|

Test Steps

List any steps necessary to trigger the feature you've created or bug you are fixing

Often it's helpful to list the different scenarios that you tested as well.

Test Steps Example

If you're working on style change to the close button you could say:

  • Works in tabs
  • Works in breakpoints pane
  • Works in autocomplete

Testing

We use husky to check the PR before it is pushed.

Here are docs on tests and linting, which you can run locally.

The integration tests will be run automatically by the CI. Our integration tests are run with mochitest. The local setup process is documented here, but the process is a bit cumbersome, so reviewers will generally help debug.

Reviews

Receiving Reviews

Once the tests have passed in the PR you must receive a review using the GitHub review system

We have a number of contributors reviewing PRs fairly quickly, if you feel yours has been neglected please mention the team name @devtools-html/debugger in the PR

Reviewing a PR

Giving valuable feedback is one of the best ways to contribute.

Tips:

  1. It's not reserved to project maintainers. In fact, it's a great way to learn about the project and pick up on conventions
  2. Don't be afraid to ask a question or comment on style inconsistencies.
  3. Ask for screenshots and steps to reproduce. Often if it's not clear to you, it's not clear to others :)

Testing locally

Testing locally is the best way to pick up on inconsistencies. Many times you'll find small things like console warnings or small visual regressions.

Steps:

  1. Find the username and branch name in the PR
  2. Add the user's remote: git remote add <usenamer> <user's fork> this is the URL you'd use to clone the user's fork.
  3. Fetch the user's branches git fetch <username>
  4. checkout the user's branch git checkout --track <username>/<pr-branch>. --track is helpful if you later want to pull down subsequent changes to the PR.

Spell Checking

We use the fabulous retext project for spell checking and other grammatical checks. If you see a spell checking error in your markdown, you can add some of the misspelled words to our dictionary in assets/dictionary.txt

Git Workflow

Working on OSS will test your git game! No matter how well you know git you're going to learn something new. Here are some links we've found useful:

  • Learn git branching - an interactive environment for learning how git commands work
  • Git Forking - Overview of creating a feature branch, keeping it up-to-date, and publishing it
  • Flight Rules - A guide about what to do when things go wrong

Merge Conflicts

It's common to create a PR and a couple days later see that it has a conflict. There are two approaches: the github ui, update your branch locally.

If the problem is simple, you can use the ui. Generally, you'll want to update your branch locally. The first thing to do is to clean up your unstaged work by either committing it, stashing it, or checking it out. Once your branch is clean, you should update your local master branch. It's a good rule of thumb that master should point to origin, but often the master branch points to your fork. If this is the case, then you'll need to add origin as a remote.

Once master is up-to-date, you can go back to your feature branch and update it. Generally the best thing to do is to rebase it against master: git rebase master, but rebases are complicated so checkout the servo, edx, and docs.

In some cases, where your feature branch has some nasty conflicts you can cherry pick your work on top of master. There are three steps:

  1. squash your new commits into one commit. (save the commit sha)
  2. reset your branch against master (temporarily wiping everything) git reset --hard master
  3. cherry-pick your commit. git cherry-pick 2bc3D

rebase-screen

Updates

We value landing PRs smoothly. One way we minimize back and forth is by pushing updates directly to PR branches.

There are a couple times when we do this:

  • it's a small syntax or style change that's blocking a merge
  • we want to suggest a refactor. At this point, feel free to offer your opinion.

Here are the steps for pushing to a branch.

CI

We use Circle for CI, which is generally pretty great. Our test run is defined in config.yml.

Testing on CI

If a test is failing on CI and you're not sure why, it can be helpful to SSH in and debug it locally. There are three steps:

  1. Rebuild with SSH
  2. copy the SSH command
  3. cd debugger.html
  4. jest src
Rebuild with SSH

SSH Command

SSH from the terminal