Differential D30589 Diff 90287 documentation/content/en/articles/committers-guide/_index.adoc

Changeset View

Standalone View

View Options

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

Show First 20 Lines • Show All 2,203 Lines • ▼ Show 20 Lines

To github.com:gvnn3/freebsd-src.git

9e5243d7b659..cf6aeb8d7dda gnn-feature -> gnn-feature

....

At this point your work is now in your branch on +GitHub+ and you can

share the link with other collaborators.

=== Landing a github pull request

ygyUnsubmitted

Done

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

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

impAuthorUnsubmitted

Done

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

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

This section documents how to land a github pull request that's submitted against the FreeBSD git mirrors at github.

While this is not an official way to submit patches at this time, sometimes good fixes come in this way and it is easiest just to bring them into a committer's tree and have them be committed from there.

ceriUnsubmitted

Done

committed

ceri: committed

ygyUnsubmitted

Done

have them be committed => have them committed?

ygy: have them be committed => have them committed?

impAuthorUnsubmitted

Done

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

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

Similar steps can be used to pull branches from other repositories and land those.

ceriUnsubmitted

Done

superfluous; suggest dropping this sentence

ceri: superfluous; suggest dropping this sentence

When committing pull requests from others, one should take extra care to examine all the changes to ensure they are exactly as represented.

ceriUnsubmitted

Done

verb missing

ceri: verb missing

ceriUnsubmitted

Done

exactly

ceri: exactly

Before beginning, make sure that the local Git repo is up to date and has the correct origins set <<keeping_current,as shown above.>>

ceriUnsubmitted

Not Done

beginning

ceri: beginning

ceriUnsubmitted

Not Done

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.

ceri: This section doesn't seem to document setting origins, unfortunately. I can see that you're…

impAuthorUnsubmitted

Done

Earlier feedback had me take it out

imp: Earlier feedback had me take it out

ceriUnsubmitted

Not Done

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

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

impAuthorUnsubmitted

Done

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

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

ceriUnsubmitted

Done

Ah, I see.

ceri: Ah, I see.

ygyUnsubmitted

Done

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

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

impAuthorUnsubmitted

Done

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

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

In addition, make sure to have the following origins setup:

debdrupUnsubmitted

Done

When pull requests from others, one should take extra care to examine all the changes to ensure they are exaclty as represented.

- Before you begin, make sure that your local Git repo is up to date and has the correct origins set <<keeping_current,as shown above.>>

+ Before begining, make sure that the local Git repo is up to date and has the correct origins set <<keeping_current,as shown above.>>

You'll also need to make sure you have the following origins setup:

debdrup:

ygyUnsubmitted

Done

setup => set up?

ygy: setup => set up?

impAuthorUnsubmitted

Done

Or omit it entirely.

imp: Or omit it entirely.

[source,shell]

debdrupUnsubmitted

Done

Before you begin, make sure that your local Git repo is up to date and has the correct origins set <<keeping_current,as shown above.>>

- You'll also need to make sure you have the following origins setup:

+ In addition, make sure to have the following origins setup:

[source,shell]

debdrup:

....

% git remote -v

freebsd https://git.freebsd.org/src.git (fetch)

freebsd ssh://git@gitrepo.freebsd.org/src.git (push)

github https://github.com/freebsd/freebsd-src (fetch)

....

Often pull requests are simple: requests that contain only a single commit.

In this case, a streamlined approach may be used, though the approach in the prior section will also work.

ceriUnsubmitted

Done

contain

ceri: contain

Here, a branch is created, the change is cherry picked, the commit message adjusted, and sanity-checked before being pushed.

The branch `staging` is used in this example but it can be any name.

debdrupUnsubmitted

Done

In this case, a streamlined approach may be used, though the approach in the prior section will also work.

- Here, we create a branch, cherry pick in the change, adjust the commit message, test and push the result.

+ Here, a branch is created, the change is cherry picked, the commit message adjusted, and sanity-checked before being pushed.

I use the branch `staging` but it can be any name.

debdrup:

This can also work when there are multiple commits without conflict, though rebasing works better when there are, as outlined below.

debdrupUnsubmitted

Done

Here, we create a branch, cherry pick in the change, adjust the commit message, test and push the result.

- I use the branch `staging` but it can be any name.

+ The branch `staging` is used in this example but it can be any name.

This can also work when there are multiple commits without conflict, though rebasing works better when there are, as outlined below.

debdrup:

ygyUnsubmitted

Done

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?

ygy: This line sounds a bit confusing to me from a reader's perspective - [what] can also work…

impAuthorUnsubmitted

Done

Will give it a shot.

imp: Will give it a shot.

Briefly, this creates a branch; cherry-picks the changes from the pull request; tests it; adjusts the commit messages; and fast forward merges it back to `main`.

The PR number is `$PR` below.

ceriUnsubmitted

Done

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

ceri: Can we avoid this term as it is confusing when compared to "git cherry-pick", unless it means…

impAuthorUnsubmitted

Done

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

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

When adjusting the message, add `Pull Request: https://github.com/freebsd-src/pull/$PR`.

[source,shell]

....

% git fetch github pull/$PR/head:staging

% git rebase -i main staging # to move the staging branch forward, adjust commit message here

% git checkout main

% git pull --ff-only # to get the latest if time has passed

% git checkout main

% git merge --ff-only staging

% git push freebsd

....

[.procedure]

====

For complicated pull requests, that have multiple commits with conflicts, follow the following outline.

ygyUnsubmitted

Done

IMO the first comma is not needed

ygy: IMO the first comma is not needed

impAuthorUnsubmitted

Done

sure

imp: sure

ceriUnsubmitted

Done

multiple

ceri: multiple

. checkout the pull request `git checkout github/pull/XXX`

. create a branch to rebase `git checkout -b staging`

. rebase that `staging` to the latest `main` with `git rebase -i main staging`

ygyUnsubmitted

Not Done

. create a branch to rebase `git checkout -b staging`

- . rebase that `staging` to the latest `main` with `git rebase -i main staging`

+ . rebase the `staging` branch to the latest `main` with `git rebase -i main staging`

. resolve conflicts and do whatever testing is needed

ygy:

. resolve conflicts and do whatever testing is needed

. merge the `staging` branch into `main` as above with

ygyUnsubmitted

Done

with?

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

ygy: with? Also, should we avoid the word "merge" here? Since IMO it means continue/finish the…

impAuthorUnsubmitted

Done

reworded.

imp: reworded.

ygyUnsubmitted

Not Done

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

ygy: Do we need to/encourage reviewing the diff again after fast-forwarding? IMO it's fine 99.9% of…

. push as above

ygyUnsubmitted

Not Done

Huh... What does this mean ;)

ygy: Huh... What does this mean ;)

This will also work when bringing branches developed elsewhere into the local tree for committing.

====

debdrupUnsubmitted

Done

. push as above

- This will also work when bringing branches developed elsewhere into your tree for committing.

+ This will also work when bringing branches developed elsewhere into the local tree for committing.

====

debdrup:

Once finished with the pull request, close it using github's web interface.

If the changes are fetched with https, this last step is the only one requiring a github account.

debdrupUnsubmitted

Done

====

- Once you are done with the pull request, close it using github's web interface.

+ Once finished with the pull request, close it using github's web interface.

This last step is the only thing you need a github account for if you grab things via https.

debdrup:

ygyUnsubmitted

Not Done

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?

ygy: If we are closing the PR on the web interface anyways (as the above sentence stated), why does…

impAuthorUnsubmitted

Done

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?

imp: You can do all the steps of fetching the changes, preparing them for commit and pushing them…

debdrupUnsubmitted

Done

Once you are done with the pull request, close it using github's web interface.

- This last step is the only thing you need a github account for if you grab things via https.

+ This last step is the only thing that requires a GitHub account for accessing things via HTTPS.

[[vcs-history]]

debdrup:

impAuthorUnsubmitted

Done

minor tweaks here.

imp: minor tweaks here.

[[vcs-history]]

== Version Control History

The project has moved to <<git-primer,git>>.

The FreeBSD source repository switched from CVS to Subversion on May 31st, 2008.

The first real SVN commit is __r179447__.

The source repository switched from Subversion to Git on December 23rd, 2020.

▲ Show 20 Lines • Show All 1,580 Lines • Show Last 20 Lines