Add manual page based on the usage in the script with a few changes and
hook it up to the build.
Details
Details
- Reviewers
markj 0mp - Group Reviewers
manpages - Commits
- rG30f78a063e0d: git-arc(1): Add manual page
Igoring as well as liberal use of mandoc.
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 36812 Build 33701: arc lint + arc unit
Event Timeline
There are a very large number of changes, so older changes are hidden. Show Older Changes
| tools/tools/git/git-arc.8 | ||
|---|---|---|
| 43 ↗ | (On Diff #83487) | |
| 44 ↗ | (On Diff #83487) | |
| 48 ↗ | (On Diff #83487) | |
| 56 ↗ | (On Diff #83487) | I would not use you in man pages. |
| 60 ↗ | (On Diff #83487) | The specification here looks to be broken, no width provided. |
| 94 ↗ | (On Diff #83487) | |
| 97 ↗ | (On Diff #83487) | These do not belong in ENVIRONMENT section. |
| 116 ↗ | (On Diff #83487) | The change above should work with .Bl -item, didn't edit in-place as phab goes crazy. Or check other man pages for EXAMPLES sections. .Ic doesn't seem to be correct macro here; if you just want bold text, use .Sy. |
| 138 ↗ | (On Diff #83487) | trailing : seems to be out of place |
| 140 ↗ | (On Diff #83487) | Sort by section, then alphabetically. |
| 142 ↗ | (On Diff #83487) | |
| tools/tools/git/git-arc.1 | ||
|---|---|---|
| 36–47 | Is there a macro which ensures that these arguments are listed on one line each? | |
Comment Actions
I'm planning to do a review of this in the coming days. I'd like to help you with the mdoc part :)
Comment Actions
Sure thing, I look forward to it :)
Most of the macro use was either inspired from mdoc(7), manual pages I've written previously, or manual pages that I consider well-written.
Comment Actions
Looks ok to me. Thanks for doing this. I would remove the corresponding text from the git-arc usage message but that can be done separately.
| tools/tools/git/git-arc.1 | ||
|---|---|---|
| 186 | This script does not get installed by default, so I'm not really sure that a history section makes much sense. At least, "appeared in FreeBSD 14.0" is a bit misleading since it technically not appear in any FreeBSD release. | |
Comment Actions
Huh, you're right, that is somewhat misleading - however, it's also not completely unique:
find /usr/src/tools/ -type f -name "*.[0-9]" -exec grep "appeared" {} +
/usr/src/tools/tools/mcgrab/mcgrab.1:program first appeared in
/usr/src/tools/tools/vimage/vimage.8:Network stack virtualization framework first appeared as a patchset
/usr/src/tools/tools/vimage/vimage.8:Stiching NLNet, integrated virtualized network stack first appeared in
/usr/src/tools/tools/ether_reflect/ether_reflect.1:program first appeared in
/usr/src/tools/tools/git/git-arc.1:utility appeared in
/usr/src/tools/tools/ioat/ioatcontrol.8:driver first appeared in
/usr/src/tools/tools/mctest/mctest.1:program first appeared insrc/tools/tools is included in the src.txz distribution set, but whether that includes being part of a RELEASE is probably outside the scope of this commit ;)
I'm more than happy to change it, but at present I'm not really sure what it should be - I'll leave it unsolved for now, until I find a good way to phrase it.
Comment Actions
| tools/tools/git/git-arc.1 | ||
|---|---|---|
| 186 | Maybe Appeared in the src tools collection in FreeBSD 14? | |
| tools/tools/git/git-arc.1 | ||
|---|---|---|
| 186 | Or perhaps not in the src tree at all, and part of devel/git or devel/arcanist instead? | |
| tools/tools/git/git-arc.1 | ||
|---|---|---|
| 186 | But it's not part of those ports. Are you suggesting the script be added to one of them instead? It is somewhat specific to FreeBSD development, at least for now, so I'm not sure whether that is the right place. | |
| tools/tools/git/git-arc.1 | ||
|---|---|---|
| 186 | I was thinking it might be added to one of those ports, yes. Thinking aloud, anyway; whatever. <shrug> | |
Comment Actions
(Please move the conversation out of inline comments)
Since it's FreeBSD specific, I'm not sure either git or arcanist upstreams will accept it - which means anyone will need ports approval to get changes made to it.
I don't see the benefit in this, personally.
Comment Actions
Great job! The content seems pretty solid. I've left some comments regarding the mdoc syntax and style.
| tools/tools/git/git-arc.1 | ||
|---|---|---|
| 33 | Perhaps this description could be simplified "git-Phabricator" doesn't look good to me, but maybe I'm just not feeling this expression. That's why I wonder if it could be simplified to make it clearer. | |
| 36–47 | Kind of. Here's a corrected synopsis: .Nm .Cm create .Op Fl l .Op Fl r Ar reviewer1 Ns Op Cm \&, Ns Ar reviewer2 ... .Op Fl s Ar subscriber1 Ns Op Cm \&, Ns Ar subscriber2 ... .Op Ar commit Ns | Ns Ar commit-range .Nm .Cm list Ar commit Ns | Ns Ar commit-range .Nm .Cm patch Ar diff1 Ns Op Cm \&, Ns Ar diff2 No .Nm .Cm stage .Op Fl b Oc .Ar branch Op Ar commit Ns | Ns Ar commit-range .Nm .Cm update .Op Ar branch Oo Ar commit Ns | Ns Ar commit-range Assuming that -b is optional to the stage verb, and branch is required. | |
| 55 | Shouldn't we stylize git as Git in such cases? | |
| 56 | "Differential revisions" seems a bit inconsistent to me. Should that be either "Differential Revisions" or "differential revisions"? | |
| 58 | ||
| 63 | I think I'd drop -offset indent. That would be consistent with style in pages like ifconfig(8). | |
| 75 | Since main is a constant in a way, I'd stylize it with the Ql macro. | |
| 86 | Perhaps this should be EXAMPLES instead? I am not sure if the order of sections is going to be correct then... | |
| 87 | This is not needed. Please see the output of mandoc -Tlint. | |
| 91 | I'd add -offset indent for every Bd in this section. | |
| 92 | Usually, we do not use Sy to make exaples bold. I'd remove them for consistency with other manuals. | |
| 97 | Typo? s/review/revision/? | |
| 110 | We should either use .Xr git-config 1 here or something like .Nm git config perhaps. | |
| 112 | Here and in other cases as well. This [bool] is not a part of a command modifier. You may also consider the format used in rc.conf(5) for types. | |
| 113 | We usually try to use the Pq macro for quoting instead of using " directly. | |
| 115 | I'd also add a note the flag of which program we reference here (I guess arc, but I am not sure). Also, the default value is missing. OTOH, the default is quite obvious so maybe we can skip that. | |
| 120 | ||
| 122 | Can we merge this and the next line into one line? | |
| 124 | Can we merge this and the next line into one line? | |
| 127 | Should it be expressed in an imperative form to match other definitions for consistency? | |
| 128 | the -v flag of arc? | |
| 167 | ||
| 172 | ||
| 173 | ||
| 174 | I wonder if we should add .An -nosplit here... I guess it's up to you. | |
| 180 | ||
| tools/tools/git/git-arc.1 | ||
|---|---|---|
| 113 | I assume you meant the Qq macro, because Pq is parentheses while the former is double quotes, so that's what I went with, since it matches the text that you rightly spotted should be a macro. :) | |