git: document how to merge a pull request from github.

[imp]

Summary

Sponsored by: Netflix

[imp]

update per jrtc27 and Darius on irc

[imp]

note closing at the end.

[debdrup]

The documentation team prefers to avoid use of 'you' in the handbook (and any other documentation).

These are just examples, though - feel free to tweak as needed.

documentation/content/en/articles/committers-guide/_index.adoc

2219

2220

2231

2232

2261

2263

2264

[ceri]

documentation/content/en/articles/committers-guide/_index.adoc

2214

committed

2215

superfluous; suggest dropping this sentence

2217

verb missing

2217

exactly

2229

contain

2234

Can we avoid this term as it is confusing when compared to "git cherry-pick", unless it means the same thing?

2252

multiple

[ceri]

As per comments.

[imp]

I don't like the passive voice, due to forced conditioning in high school Freshman English.... however, I know that's the style here...

documentation/content/en/articles/committers-guide/_index.adoc

2234

It means exactly the same thing as 'git cherry-pick' in this context.

2264

minor tweaks here.

[imp]

review feedback

[ceri]

documentation/content/en/articles/committers-guide/_index.adoc

2218

beginning

2218

This section doesn't seem to document setting origins, unfortunately. I can see that you're reusing text from another section though, so probably a bit harsh to reject this on that basis - unless you want to fix that at the same time, I'll take a separate PR to fix that.

[imp]

documentation/content/en/articles/committers-guide/_index.adoc

2218

Earlier feedback had me take it out

[ceri]

documentation/content/en/articles/committers-guide/_index.adoc

2218

I may be doing something wrong, but I still see it when I download the diff.

[imp]

documentation/content/en/articles/committers-guide/_index.adoc

2218

I had the whole origin / fetch stuff in as commands and I took it out.

[imp]

I do think the origin setting stuff should be in #keeping_current link instead... and I'd prefer to improve that async to this review.

[ceri]

In D30589#687275, @imp wrote:

I do think the origin setting stuff should be in #keeping_current link instead... and I'd prefer to improve that async to this review.

Yeah, happy with that.

documentation/content/en/articles/committers-guide/_index.adoc

2218

Ah, I see.

[ceri]

Just the “beginning” typo then lgtm.

[debdrup]

In D30589#687207, @imp wrote:

I don't like the passive voice, due to forced conditioning in high school Freshman English.... however, I know that's the style here...

Believe me, I'm no more fond of it than you, but The Style is The Style, if you'll permit me to be tautological. :)

[imp]

final(?) typos

[imp]

@ceri @debdrup ok. I think this is ready to land.

[ygy]

The content looks good to me, just some minor nits.

documentation/content/en/articles/committers-guide/_index.adoc

2212

I'm wondering if we need to use "GitHub" instead of github...

2214

have them be committed => have them committed?

2218

Similarly, we should probably settle git vs Git across the doc.

2219

setup => set up?

2232

This line sounds a bit confusing to me from a reader's perspective - [what] can also work, without conflict [against what], when there are [?]. Perhaps breaking into two sentences and rephrasing it?

2251

IMO the first comma is not needed

2257

with?

Also, should we avoid the word "merge" here? Since IMO it means continue/finish the rebase here.

[imp]

documentation/content/en/articles/committers-guide/_index.adoc

2212

We don't currently, and such a change would be beyond the scope of this change.
I'll change it here though.

2214

-> "and have them pushed into the FreeBSD's tree from there."
might be better. what do you think?

2218

I tried to do that in the draft docs before we landed them into the tree, so I'll do this here.

2219

Or omit it entirely.

2232

Will give it a shot.

2251

sure

2257

reworded.

[imp]

ygy

[ygy]

documentation/content/en/articles/committers-guide/_index.adoc

2255

2257

Do we need to/encourage reviewing the diff again after fast-forwarding? IMO it's fine 99.9% of the time but... (question mark)

2258

Huh... What does this mean ;)

2263

If we are closing the PR on the web interface anyways (as the above sentence stated), why does "requiring an account" depend on HTTPS or not?

[imp]

more feedback fdrom ygy

All but the last thing.

[ygy]

Thanks for making all the changes! This looks good to me, pending other reviewers' approvals.

[imp]

documentation/content/en/articles/committers-guide/_index.adoc

2263

You can do all the steps of fetching the changes, preparing them for commit and pushing them into the FreeBSD w/o a github account.
To close the pull request at the end, though requires a github account.

Is there a better way to say that?