Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update CONTRIBUTING.md #9255

Merged
merged 4 commits into from
Feb 6, 2020
Merged

Update CONTRIBUTING.md #9255

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9fbb93a
Update CONTRIBUTING.md
Meekohi Feb 1, 2020
6103fdf
Update CONTRIBUTING.md
Meekohi Feb 4, 2020
5578678
Update CONTRIBUTING.md
Meekohi Feb 4, 2020
596b798
Update CONTRIBUTING.md
Meekohi Feb 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ The conventions for module exports are:
### Version Control Conventions

* We use [rebase merging](https://git-scm.com/book/en/v2/Git-Branching-Rebasing) (as opposed to [basic merging](https://git-scm.com/book/en/v2/Git-Branching-Basic-Branching-and-Merging#Basic-Merging)) to merge branches
* To rebase your fork's PR branch onto master, the recommended approach is `git rebase origin/master`, `git push --force origin your-branch`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is helpful, but do you think we should also drop a note about branch names? eg. it's not clear what origin is here when you have a fork depending how you've setup your local repo, since you'll likely have one remote for your fork and a remote for upstream (mapbox/mapbox-gl-js)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah good point it should really be upstream/master for most people... Maybe it's worth saying full details on the "preferred setup", e.g.

  1. Fork this project
  2. Clone your new fork, git clone [email protected]:GithubUser/mapbox-gl-js.git
  3. cd mapbox-gl-js
  4. Add the Mapbox repository as upstream: git add remote upstream [email protected]:mapbox/mapbox-gl-js.git
  5. Create a new branch git checkout -b your-branch for your contribution
  6. Write code
  7. When you need to rebase, git fetch upstream, git rebase upstream/master and force push to Github git push --force origin your-branch

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be sufficient in this instruction to add a note for downstream forks:

Suggested change
* To rebase your fork's PR branch onto master, the recommended approach is `git rebase origin/master`, `git push --force origin your-branch`
* To rebase your PR branch onto master, the recommended approach is `git rebase origin/master`, `git push --force origin <your-branch>`. When working with a fork, `origin/master` should point to the upstream `master` branch.


## Documentation Conventions

Expand Down