Page MenuHomeFreeBSD

Igor the shit out of the committer's guide.
ClosedPublic

Authored by mat on Aug 16 2016, 3:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, May 2, 5:51 PM
Unknown Object (File)
Thu, May 2, 5:50 PM
Unknown Object (File)
Thu, May 2, 5:49 PM
Unknown Object (File)
Thu, May 2, 2:26 PM
Unknown Object (File)
Thu, May 2, 2:24 PM
Unknown Object (File)
Thu, May 2, 1:25 PM
Unknown Object (File)
Feb 28 2024, 11:44 AM
Unknown Object (File)
Feb 1 2024, 8:08 AM
Subscribers

Details

Summary
  • "(s)" used 5 times
  • "in order to" used 6 times
  • "i.e." used 1 time
  • "e.g." used 1 time
  • "simply" used 3 times
  • "basically" used 2 times
  • "the following" used 18 times
  • "as follows" used 3 times
  • "should" used 83 times

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

I've found a few things that should be improved, but overall, good work. +1 for the title of the review. ;-)

en_US.ISO8859-1/articles/committers-guide/article.xml
1827

I think there is a word missing now after the "as". Something like "as it" or "as they".

2025

I think you can even drop the "these" here.

4108

s/contains/contain/

mat marked 3 inline comments as done.

Update with @bcr's comments.

Wow!

en_US.ISO8859-1/articles/committers-guide/article.xml
480

Can remove that indefinite "It":

New committers are assumed to already be familiar...
1003

Passive -> active:
s/will show/shows/

1263

No "s" because of the plural "merges":
s/looks/look/

A note on "should": it's not always wrong. The wrong way to use it is when it implies the writer is not sure of what they are saying, like "the screen output should look like this". The right way is when you are making a recommendation: "You should always use strong passwords."

In this case, you've made it more imperative. It's not a recommendation, it's the way things are. Nice!

1269

How about "must be" rather than "is"?

1272

As above, probably better with s/$/must/

1289

Redundant with the same word used in the next line. How about "process"?

1299–1300
directory where changes are being merged)
1317

It would be terrible if something were to happen and this old, broken, confusing merge process were to be accidentally deleted totally by accident and unintentionally.

1460

Better with "For example, to merge:"

1615

Nice!

1630

As above, probably better as "must be".

1784

This is one of those rare instances where "should" is appropriate. "can" makes it sound like it is not possible rather than something that is to be avoided.

1787

Since we are here, break sentence:

Committing is now possible.  However, it is good
1788

s/OK/okay/

1827
conflicts as they will be handled manually.</para>
1871

Good, but the original sentence is poor and the writer probably misused i.e. when they mean e.g., another reason to avoid these.
Easy to split the sentence here:

vendor tree but not in the main tree.  These are things that
1873

Redundant "like" and "for example".

import, like the vendor's <filename>Makefile</filename>s
2014–2016

Hard to keep track of what "it" means here. Rearrange:

Avoid setting up a <application>svnsync</application> mirror
unless there is a very good reason for it.
2016

"many" and "multiple" are redundant. Remove "many".

s/the network/network/

2020

Spell out numbers up to ten:

four to ten
2025

I would keep "these" but lose "three", reason being that counts in text frequently are wrong after text is edited.

Or better, just say

There are several ways to do this:</para>
2085

Missing "name" after "same".

2128

s/adding to/adding these entries to/

2262

"your" is okay here, because it is specific. Sometimes these are hard to avoid.

2391

The original sentence is kind of misleading. Reading it does not mean it will be kept. How about

or save it elsewhere.</para>
2394

s/processing/processing,/

2400

s/email/email,/

2403

End the sentence here:

class="fqdomainname">freefall.FreeBSD.org</systemitem>.</para>

The rest of the sentence is redundant.

2468

s/commit/commit,/

2582

Hm. The original is a complex and confusing sentence which appears to include irrelevant information.

The commit is from a PR submitted by John Smith.
2584
The commit message <quote>PR</quote> and <quote>Submitted by</quote> fields are filled.
2595–2596
<para>The virtual memory system is being changed. After
2608–2609

<para>Commit a port, after working with```

2624
<para>Commiting some code based on work done in the
2636

<para>Committing some code which will be merged from```

2652
It is often necessary to combine these.
2654

This can be third-person:

Looking at the PR, the developer sees it is not an area of the tree they normally work in,
so they have the change reviewed by the <literal>arch</literal> mailing list.  Since the
change is complex, the developer opts to <acronym>MFC</acronym> after one month to
allow adequate testing.</para>
2680

"Currently" isn't doing anything useful here.

s/Currently the/The/

2812

s/When working on/Working on/

2815

s/When trying/Trying/

2818

s/you/a developer/

2820–2821

s/you/a developer/

2823
repository, if it s not clear who the active maintainer
2824

s/be,/is,/

2829

s/your queries/queries/

2839
<para>If there is any doubt about a commit for any reason at all, have
2842
part of the repository. If a commit does
2843
does result in controversy erupting,
2845

Remove that misused, ugly emdash:

settled. Remember, with a version control system we can
2848
<para>Do not impugn the intentions of others.
2849
If they see a different solution to a problem, or even
2850
a different problem, it is probably not because they are stupid, because
2876
<para>When unsure about something, whether it be a
2898

s/will/can/

3142
Now &man.ssh-add.1; can be used for
3143
authentication once per session. It prompts for
3144
the private key's pass phrase, and then stores it in the
3145
authentication agent (&man.ssh-agent.1;). Use
<command>ssh-add -d</command> to remove keys stored in the agent.<para>
3149
Test with a simple remote command: <command>ssh
3385
Phabricator service.  The commit will only happen once
3387
does not mean that permission is required before
3389
misspelling, just that it is good to develop a feel
3395
The very best way of making sure that things are on the right
3396
track is to have code reviewed by one or more other
3419
look at the repository logs for the files
3501–3502
cooled back down. Do not air
3502
angry words in public and do not forward private
3509
the person sending a flame-o-gram at least had the
3601

This whole paragraph needs a rewrite.

3679

s/theses/these/

3772

s/are not/must not be/

3786

s/are/must be kept/

3791–3792
are considered private communications.
3792
Permission is required to publish material from these
3948

This is a recommendation, so either "should be" or "must be", depending on how imperative you feel.

3965

s/are/is/ (singular "support")

4275
subdirectory to another, or when the
4276
name of a directory must be changed because the authors renamed their
4619
<para>This is much simpler than a physical category.  Only
4620
a few modifications are needed:</para>
4699

"a port's" is redundant and can be removed.

mat marked 81 inline comments as done.May 31 2017, 12:03 PM
mat added inline comments.
en_US.ISO8859-1/articles/committers-guide/article.xml
1317

That it would.

3601

I'll make a note of it for later.

This revision was automatically updated to reflect the committed changes.
mat marked 2 inline comments as done.