Differential D30589 Diff 90353 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,202 Lines • ▼ Show 20 Lines

remote: Resolving deltas: 100% (1/1), completed with 1 local object.

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

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 pushed into the FreeBSD's tree from there.

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

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

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

In addition, make sure to have the following origins:

[source,shell]

....

% 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.

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.

This technique works for any number of commits in the pull request, especailly when the changes apply cleanly to the FreeBSD tree.

However, when there's multiple commits, especially when minor adjustments are needed, `git rebase -i` works better than `git cherry-pick`.

Briefly, these commands create 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.

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.

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

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

. rebase the `staging` branch 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

. fast forward the `staging` branch into `main` as above

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…

. final sanity check of changes to make sure all is well

ygyUnsubmitted

Not Done

Huh... What does this mean ;)

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

. push to FreeBSD's Git repository.

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

====

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

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…

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

[[vcs-history]]

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…

== Version Control History

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…

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

ceriUnsubmitted

Done

superfluous; suggest dropping this sentence

ceri: superfluous; suggest dropping this sentence

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

ceriUnsubmitted

Done

verb missing

ceri: verb missing

ceriUnsubmitted

Done

exactly

ceri: exactly

The first real SVN commit is __r179447__.

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.

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

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.

The last real svn commit is __r368820__.

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:

The first real git commit hash is __5ef5f51d2bef80b0ede9b10ad5b0e9440b60518c__

The FreeBSD `doc/www` repository switched from CVS to Subversion on May 19th, 2012.

The first real SVN commit is __r38821__.

The documentation repository switched from Subversion to Git on December 8th, 2020.

The last SVN commit is __r54737__.

The first real git commit hash is __3be01a475855e7511ad755b2defd2e0da5d58bbe__.

The FreeBSD `ports` repository switched from CVS to Subversion on July 14th, 2012.

ceriUnsubmitted

Done

contain

ceri: contain

The first real SVN commit is __r300894__.

The ports repository switched from Subversion to Git on April 6, 2021.

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:

The last SVN commit is __r569609__

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.

The first real git commit hash is __ed8d3eda309dd863fb66e04bccaa513eee255cbf__.

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.

[[conventions]]

== Setup, Conventions, and Traditions

There are a number of things to do as a new developer.

The first set of steps is specific to committers only.

These steps must be done by a mentor for those who are not committers.

[[conventions-committers]]

=== For New Committers

Those who have been given commit rights to the FreeBSD repositories must follow these steps.

* Get mentor approval before committing each of these changes!

* All [.filename]#src# commits go to FreeBSD-CURRENT first before being merged to FreeBSD-STABLE. The FreeBSD-STABLE branch must maintain ABI and API compatibility with earlier versions of that branch. Do not merge changes that break this compatibility.

[[commit-steps]]

[.procedure]

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

*Procedure 1. Steps for New Committers*

. Add an Author Entity

[.filename]#doc/shared/authors.adoc# - Add an author entity. Later steps depend on this entity, and missing this step will cause the [.filename]#doc/# build to fail. This is a relatively easy task, but remains a good first test of version control skills.

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.

. Update the List of Developers and Contributors

[.filename]#doc/documentation/content/en/articles/contributors/contrib-committers.adoc# - Add an entry, which will then appear in the "Developers" section of the link:{contributors}#staff-committers[Contributors List]. Entries are sorted by last name.

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:

[.filename]#doc/documentation/content/en/articles/contributors/contrib-additional.adoc# - _Remove_ the entry. Entries are sorted by first name.

. Add a News Item

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:

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.

[.filename]#doc/website/data/en/news/news.toml# - Add an entry. Look for the other entries that announce new committers and follow the format. Use the date from the commit bit approval email from mailto:core@FreeBSD.org[core@FreeBSD.org].

. Add a PGP Key

`{des}` has written a shell script ([.filename]#doc/documentation/tools/addkey.sh#) to make this easier. See the https://cgit.freebsd.org/doc/plain/documentation/static/pgpkeys/README[README] file for more information.

Use [.filename]#doc/documentation/tools/checkkey.sh# to verify that keys meet minimal best-practices standards.

After adding and checking a key, add both updated files to source control and then commit them. Entries in this file are sorted by last name.

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