Forklift reorganization and significant rewrite of the FreeBSD Committer's Guide
Diff Detail
- Repository
- rD FreeBSD doc repository - subversion
- Lint
No Lint Coverage - Unit
No Test Coverage - Build Status
Buildable 8021 Build 8197: arc lint + arc unit
Event Timeline
A lot of remarks and suggestions. Overall, I'm happy with how this is being restructured.
en_US.ISO8859-1/articles/committers-guide/article.xml | ||
---|---|---|
84 | s/tecnological/technological/ | |
89 | I'm not sure whether this is still the case. You might need to ask. I believe they are owned and operated by IXsystems now. | |
98 | s/cloked/cloaked/ | |
137 | I would remove "possibly" as it is always the case to mentor someone for a certain commit bit. | |
152 | src committers can not commit to doc, they need a doc bit for that. With approval and review by someone that has a bit for that area, everyone may commit to that area. For example, a ports committer may commit a change for the porters handbook when this change was approved by someone with a doc bit. | |
158 | doc is special in this: doc committers may make textual changes like typo fixes to comments and user visible output in src and ports (although the latter is even less common), but may not do any code changes. Even when a textual change is to be made, it is good practice to ask the responsible person (see MAINTAINERS file in the src repo) for permission. If no maintainer is listed, then ask the person that made the latest change to the code/file in question. | |
222 | This is wrong. Mentored committers must have an "Approved by:" line in their commits to indicate that their mentor (or another person with the same full commit bit) has seen and approved the commit. | |
292 | You can break up this long line after "<link" and indent the following "xlink..." on the next line. Also, you can do a regular line break after "Installing" in the link description. | |
302 | I would write "... with your real name ..." | |
326 | You can move the <xref part of the link to the line above and indent the "linkend= ..." on this line. | |
348 | Maybe add <literal> tags around !ENTITY? | |
350 | Definitely add <literal> tags around !ENTITY here. | |
360 | The ... belong to the line where the <screen> tag is. | |
364 | This </userinput> tag should not be alone on a line by itself. It should close the last line of text you are displaying that way. | |
374 | Maybe you can break this up into "... of <filename>contrib.committers.xml</filename>, located in <filename>doc/head/.../articles</filename> to appear ..." | |
379 | s/screen/userinput/ | |
385 | Like above, this needs to be on the line where the last ... is. | |
387 | <literal>!ENTITY</literal> | |
396 | Sentence stop at the end is missing. | |
399 | The commit bit email will not come from core for doc or ports bits. I would drop this part to just say "Use the date from the committ bit approval email." | |
401 | Use <userinput> here, again. | |
412 | Replace (doc) with commit bit type (in <replaceable> tags). | |
426 | Move the <xref part to the line above and indent the "linkend=..." part on this line. | |
438 | xlink needs to be indented. | |
439 | Two spaces after . This probably requires a separate sweep through your changes as it may appear in multiple locations. | |
443 | Not every new user will have multiple keys, so not sure whether we should include this line. | |
515 | there seems to be a word missing after Kerberos. | |
525 | Insert a line break after <link and indent the xlink: line. | |
527 | with a non-&os; | |
530 | Add a space after "Mail" | |
534 | s/Keberos/Kerberos/ | |
543 | Avoid using the links at beginning of sentences. "Event organization such as DevSummits is also done via the wiki." | |
553 | Pull the <link parts up into the previous line in all the links here and indent the lines after it. | |
600 | You can also use your jru example user here. | |
763 | s/pass phrase/passphrase/ | |
769 | /usr needs to be wrapped into <filename> tags. | |
806 | you need to break the line after "with" and keep the <filename> part on its own line. | |
1128 | Try starting this sentence with an uppercase letter by rephrasing it. | |
1163 | Same as above, start with uppercase by rephrasing. | |
1168 | you need to indent this line. | |
1269 | check the indentation in this <para>, &a.developers; would certainly fit in the line above. | |
1723 | Nowadays, running mandoc -Tlint is recommended to catch bugs in man pages. | |
1734 | Mixing whitespace with content changes is less of a problem now that we have the PO system for translators. | |
1735 | Superfluous space before sentence stop. | |
2200 | Yes, the developers handbook might be a better place for it, as it is more relevant for people with src bits. | |
4319 | Could also be in the developers handbook. doc committers don't need to know this. |
Had to stop here, not even a quarter of the way through. This is too large a change. Please consider breaking it up into changes to individual sections that can be reviewed more easily.
en_US.ISO8859-1/articles/committers-guide/article.xml | ||
---|---|---|
3 | This should not change. Not sure why it did. | |
12 | s/FreeBSD/&os;/ | |
38 | This is a whitespace change. Phabricator is helpfully making sure I can't really tell which kind, but possibly a blank line with just a tab. Whitespace changes are bad, mkay? | |
58 | Use the serial/Harvard/Oxford comma: | |
60 | The "while not" construct is kind of esoteric. Consider non-native English speakers and translators. Also, "will apply" is kind of passive. The first half of this sentence can possibly be removed: `Every project participant...` | |
67 | Indent error. Because this is inside a new tag, needs another level of indent: refer to the <link xlink:href="http://www.freebsd.org/internal/new-account.html">New Account Creation Procedure</link> page for guidance.</para> | |
73 | The introduction? Probably. | |
127 | This sentence is really complicated. Please keep it simple. Some of us were edumacated in American schools. That also keeps it simple for non-native English speakers and translators. | |
131 | This sentence can be simpler: "Commit bits are specific for certain parts of the repository." Although strictly speaking, we have three different repos. | |
133 | Not sure what this means. "The areas associated with a bit"... | |
134 | The colon is just a sentence splice here. When possible, use two shorter sentences rather than a long one spliced with something. Colons should introduce a list or conclusion. Semicolons are generally for fictional narratives. | |
137 | This whole sentence is kind of fuzzy, suggesting the reader follow the right procedures but not giving any guidance as to what they are. | |
178 | Please simplify this. Use small words and short sentences. It's not that the reader is stupid, it's that they have a lot going on. Also not sure about the word "tree". I think we mainly refer to repositories here and elsewhere, and should be consistent. Also also, be careful with "should" and "may". "May" generally implies permission: "You may now delete that code." while "should" is preferably only for recommendations: "But you should make a backup of it first." (And yes, avoid "you" and "your" as informal. I know I did it just there. And I'd do it again.) | |
181 | The second half of this sentence is not needed. | |
185 | "Tree" again. | |
197 | s/doc/Documentation/ | |
199 | s/such as/including/ But I'm not certain about whether enumerating all the different types of files is useful here. Could say "Documentation committers can commit spelling, grammar, and text clarification fixes to the src repository." (although recommend review from a src committer). | |
207 | I know what "non-mentored committer" means, but will a new committer? Is there a non-negative logic version of this? | |
215 | 'access' is a file, so should use <filename> tags and probably have more of a path. | |
216 | s/ensue/begin/ But I think this is another section where shorter, simpler sentences would help. | |
221 | I'm not sure about just "src", seems like that should be marked up in some way. Likewise with "Approved by", which should probably be in <literal> tags. | |
222 | Do not use "--", which is more properly an emdash. But don't use emdashes either, split the sentence. Short, declarative sentences. Also, markup for literal text. | |
292 | Yes. Also, if the entire <link></link> line will fit on a single line, can move it to a line of its own for readability. | |
293 | s/FreeBSD/&os;/ | |
294 | Missing an ending period on the line above, and the </para> needs to be against that rather than on a new line. | |
298 | I'm not sure about the word "setup" here. | |
300 | Please don't use quotes like this. There are markup tags for email addresses, and ways to use them that do not result in a mailto link: stupid phabricator link I would also say not to use freebsd.org as the mail domain, but rather example.org. | |
302 | Telling the user to replace this information with the real information is probably not necessary. Either way, any information to be replaced must be in <replaceable> tags. | |
304 | Passive: s/Those who have been given commit rights/New committers/ | |
310 | s/!/./ | |
317 | The literal tags look like an artifact. | |
326 | You can... but this is better because it keeps the whole thing on one line for readability. | |
352 | <acronym> | |
354 | s/missing/skipping/ | |
357 | Need tags on the domain, and it should be capitalized (@FreeBSD.org). | |
360 | Well... <screen> is for screen output, and <userinput> is used inside that to show what the user types. But these are contents of files, and for that all you need is a <programlisting> element. | |
372 | This is not a quotation, but a literal text entry. | |
379 | <programlisting>, rather. | |
389 | Not <quote>. |
wblock,
Yes, this is a forklift upgrade of a ransom note of a document. Please specify the sequence of changes you would like to see.
I observe:
- Sequence. Sections were added by random people in random places over the years.
- The few commenting out of sections that do not belong there (IMHO).
- Clean up of the existing text where appropriate (though I have not reviewed large swaths of it such as the embedded SVN tutorial)
- New content that made the document usable. I addressed all of these at once at AsiaBSDCon because of the access I had to people like my mentor
Please advise.
News: I confirmed that the FreeBSD Mall offer is still valid and have news on an MSDN offer.
I would go through each of the comments made so far and fix them one by one, providing a new patch for review in the process. The SVN guide has been added from the wiki many years after the original committers guide was written and used (remember, we were on CVS back then). This one has been independently reviewed and as far as I can tell, Michael only rearranged it to be more consistent with the flow of the document, but did not do any content changes on it. So we should take care that the actual committers guide gets updated to reflect reality.
en_US.ISO8859-1/articles/committers-guide/article.xml | ||
---|---|---|
3941–3942 | There have been some changes to this content recently that will need to be merged in, although looking at a broader scope I wonder if we shouldn't move this out to a separate document, and just reference it from here? There are FreeBSD consumers (e.g. downstream projects and contributors) who are not committers, but who still have an interest in the meaning of the tags. Or it might just be that we should make sure they're aware of the content in this guide. |
Ed's comments are all accurate. This document conflates many things and suggest we take a moment to address them at BSDCan or in advance. I for one think the nuts and bolts of SVN be separated out, ideally in a way that produces a document that can double as an in-base man page for snvlite(1).