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.

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

Document branches in contributing guidelines #1233

Merged
merged 2 commits into from Dec 4, 2018

Conversation

pavolloffay
Copy link
Member

Also resolves #1232

Signed-off-by: Pavol Loffay <ploffay@redhat.com>
@codecov
Copy link

codecov bot commented Dec 3, 2018

Codecov Report

Merging #1233 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1233   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         159     159           
  Lines        7136    7136           
======================================
  Hits         7136    7136

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d421a00...4f28656. Read the comment docs.


```
git config --add alias.amend "commit -s --amend"
git config --add alias.c "commit -s"
Copy link
Member

Choose a reason for hiding this comment

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

Do these actually work? I tried once but it didn't help.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will try, if they don't work I will remove them.

Copy link
Member Author

Choose a reason for hiding this comment

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

Does not work for me I will remove it

Signed-off-by: Pavol Loffay <ploffay@redhat.com>
@pavolloffay pavolloffay merged commit 6237e2f into jaegertracing:master Dec 4, 2018
@ghost ghost removed the review label Dec 4, 2018
@pavolloffay
Copy link
Member Author

@yurishkuro @vprithvi @black-adder @tiffon please do not push feature branches to upstream and rather submit PR from your fork.

https://github.com/jaegertracing/jaeger/branches/all

@yurishkuro
Copy link
Member

I have already changed my git clone.

## Branches

Upstream repository should contain only maintenance branches (e.g. `release-1.0`). For feature
branches use forked repository.
Copy link
Member

Choose a reason for hiding this comment

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

We may want to add a bit of explanation here. In particular, one critical step that does not happen automatically with a fork is to make sure the local master branch tracks the upstream, not your fork (origin).

$ git remote -v
origin	git@github.com:jaegertracing/jaeger.git (fetch)
origin	git@github.com:jaegertracing/jaeger.git (push)

$ git remote rm origin

$ git remote add upstream git@github.com:jaegertracing/jaeger.git

$ git remote add origin git@github.com:yurishkuro/jaeger.git

$ git fetch upstream master
remote: Enumerating objects: 13, done.
remote: Counting objects: 100% (13/13), done.
remote: Total 19 (delta 12), reused 13 (delta 12), pack-reused 6
Unpacking objects: 100% (19/19), done.
From github.com:jaegertracing/jaeger
 * branch            master     -> FETCH_HEAD
 * [new branch]      master     -> upstream/master

$ git pull
There is no tracking information for the current branch.
Please specify which branch you want to merge with.
See git-pull(1) for details.

    git pull <remote> <branch>

If you wish to set tracking information for this branch you can do so with:

    git branch --set-upstream-to=<remote>/<branch> master

$ git branch --set-upstream-to=upstream/master master
Branch 'master' set up to track remote branch 'master' from 'upstream'.

$ git remote -v
origin    git@github.com:yurishkuro/jaeger.git (fetch)
origin    git@github.com:yurishkuro/jaeger.git (push)
upstream  git@github.com:jaegertracing/jaeger.git (fetch)
upstream  git@github.com:jaegertracing/jaeger.git (push)

Without this, the local master tracks for fork and you need to do extra steps to merge upstream master into it. People often forget to do that and end up with PRs having all kinds of weird commits in them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Contributing guidelines readme overlaps with contributing
2 participants