Send Pull Request from branch, not from master.

[rlovtangen]

If not you risk being out of sync with upstream master forever. Also, if you add more changes to master they will all be part of the same Pull Request.

[jeffscottbrown]

If we are going to be prescriptive about this, instead of something like
git push origin mine I would rather recommend something like
git push origin issue_123 and describe that issue_123 could be anything
but could reference the GitHub issue number that the change is relevant to. The
change here is apparently addressing people who don't know what they are doing
with git so I would not assume they will know the implications of using mine for
multiple unrelated changes (like if they are submitting multiple unrelated
pull requests).

[rlovtangen]

Yes, issue_123 is probably a better example than mine because it illustrates that each pull request should have its own branch.
The most important, though, is to get rid of the advice to send pull request from master.
Will you change mine to issue_123 and comment that the name could be anything, or should I?

[jeffscottbrown]

The most important, though, is to get rid of the advice to send pull request from master.

To be honest, I don't care what the branch name is that the PR is coming from. If it is called master or issue_123, the process all works exactly the same. A problem arises if they add more commits to the branch after sending the PR but that problem is the same if the branch is called master or if the branch is called issue_123. Since the documentation is addressing folks who are not familiar with how to use git then I agree that we should recommend something other than master. My real preference would be to not have our own documentation on how to effectively use git and GitHub but instead point to some canonical reference which describes how to submit pull requests. If every project on GitHub had their own documentation for how to do these things that would be a waste of a lot of people's efforts. However, as a practical matter I think it may be helpful to have at least the basics in our own documentation so I don't oppose including these things.

Will you change mine to issue_123 and comment that the name could be anything, or should I?

If you would make the change, that help would be appreciated.

Thanks for all of the help.

[rlovtangen]

Yes I know that master is just a name for a branch like anything else, but the documentation suggested to merge mine back to the branch (master) which tracked the upstream master, which is most likely the target for the Pull Request. So it's probably best to not trick people into doing that mistake but rather just send the PR from the branch created for the fix. (I've done that mistake myself in my early days of contributing.)

I will add the proposed change :)

[rlovtangen]

Hmm, I'm not a git/GitHub expert myself. patchesCore.gdoc in my branch is now one change behind master because of 6d9c986.
Would I rebase this branch before adding more to it, even though I have pushed it to my fork and sent a PR already, or will that make a mess?

[jeffscottbrown]

@rlovtangen This is part of the reason that I am not a fan of defining prescriptive instructions like those being edited here. Knowing how to use git should be a pre-cursor to using git.

One way to go about this is to create a local branch that is consistent with the current master branch. Make your changes on that local branch and then do something like this to push your changes to a newly created remote branch (assuming that your remote is called "origin").

git push origin name_of_local_branch:name_of_remote_branch_that_you_want_to_create

That is the most fool proof way. If you know what you are doing with rebase then you have other options. How you have things configured in ~/.gitconfig may also be a factor (for example, I have some things configured related to tracking branches that simplies these kinds of things for me).

I hope that helps. If it doesn't just let me know and I will make the changes.

Thanks again for all of the help.

[rlovtangen]

Not sure I followed how I could get additional changes as part of this PR if I'm pushing to a new branch, and why the remote name should be different. Maybe you meant to do the changes again and submit a new PR instead? I need to read up on this, maybe. I use Git at work, but not GitHub, and especially not pull requests as we all have full access to our repos.

Anyway, I added the change as an additional commit to this PR. Hope that's fine. The change in 6d9c986 is on a non-conflicting line, so the merge should be automatic. When sending parallel pull requests that operates on the same files, like I did, one of the pull requests would need to face this anyway.

BTW: I'm not a native English speaker, so I won't be upset if you or others make grammar changes or rewrite whole sentences to something better. That would be quite good, actually. I really care about having good updated documentation, so I won't let the fact that I'm not always able to produce perfect English, stop me from contributing. Hope that's OK :)