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

2273

2274

2285

2286

2315

2317

2318

[ceri]

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

2268

committed

2269

superfluous; suggest dropping this sentence

2271

verb missing

2271

exactly

2283

contain

2288

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

2306

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

2288

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

2318

minor tweaks here.

[imp]

review feedback

[ceri]

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

2272

beginning

2272

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

2272

Earlier feedback had me take it out

[ceri]

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

2272

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

2272

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

2272

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

2266

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

2268

have them be committed => have them committed?

2272

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

2273

setup => set up?

2286

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?

2305

IMO the first comma is not needed

2311

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

2266

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

2268

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

2272

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

2273

Or omit it entirely.

2286

Will give it a shot.

2305

sure

2311

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?