- User Since
- Jan 12 2018, 3:33 PM (203 w, 4 d)
Sun, Dec 5
Would it be an idea to refer to this manual page from boot(9), boot(8), and/or some other relevant manual pages - to make it easier to discover?
Thu, Dec 2
Tue, Nov 30
This landed with commit bd6dce978c1a, but I forgot to add the differential revision tag.
Sat, Nov 27
I think it'd be nice to get all other functions marked up properly too, unless they're part of display blocks (Bd Ed macros in mdoc).
Fri, Nov 26
Address feedback by PauAmma
Mon, Nov 22
The proper way to solve this would be to use /usr/bin/env $BINARY because if that doesn't work, the OS that the tree is being built on isn't POSIX-compliant at all.
Thu, Nov 18
This looks good to me.
Wed, Nov 17
Tue, Nov 16
There's only one minor nit, but otherwise it looks good to me.
Mon, Nov 15
Sat, Nov 13
Are there other places where -(j|J) -1 is used to list all jails, instead of simply doing -(j|J)?
Also, to channel bcr:
Remember to bump .Dd :)
Wed, Nov 10
Nov 3 2021
Try to address feedback by a.wolk
Oct 16 2021
I don't think enough context was included, because I'm still seeing "Context not available" - any chance you can use arcanist from ports/packages (as described on the wiki article I linked earlier) or src/tools/tools/git/git-arc.sh (which can be installed by doing make -C /usr/src/tools/tools/git/ install (which requires arcanist, git, and textproc/jq?
Could you please update the diff to one made using the instructions on the Phabricator wiki article?
Oct 14 2021
This is only true if you use format-patch?
I believe you need to use git-am(1) to get the metadata (author, timestamps, et cetera) applied.
Oct 13 2021
Here's a few things I noticed.
Oct 3 2021
For the record, the mdoc looks good to me now.
I can't speak to the code.
Did the item macros disappear by choice? Just checking.
Oct 2 2021
Just a minor nit, otherwise it looks good to me.
I noticed that there are several places where a phrase is entirely or almost entirely repeated, and in my experience that's the sort of thing that makes it hard to scan through a manual page quickly - so I think it might be a good idea to rephrase things a bit.
Ideally I'd also really like to see the words include and exclude be used, as they're the kind of keywords that one would look for when dealing with this kind of thing.
Sep 29 2021
Sep 3 2021
I've given it another once-over, and am pretty happy with it, so unless anyone else has any interjections, I say it's good to go.
Aug 30 2021
Aug 29 2021
Aug 28 2021
These are a few things I spotted, but I'm not sure I caught everything and I'm on the wrong side of entirely sober. :)
Can you switch this diff to one with context? It makes it easier to make suggestions. :)
Aug 16 2021
Jul 24 2021
Jul 23 2021
Jul 22 2021
Jun 29 2021
I clearly shouldn't review things when I'm tired. Sorry. :(
Seems like an excellent idea.
Jun 11 2021
I don't really have time for this right now, so someone else can go ahead and commit it once it's been accepted.
Jun 1 2021
The documentation team prefers to avoid use of 'you' in the handbook (and any other documentation).
May 18 2021
Have you tested with textproc/igor and mandoc -T lint?
Address feedback by 0mp
The current diff you've uploaded lacks context for Phabricator to tell which part of the file it applies to (I don't know why Phabricator can't always figure out the context), which is why the "Context not available" message appears.
Yes, it would be good to have a documented method of checking the protected flag, so go ahead and add that.
Fix cross reference
Address feedback by ceri
Address feedback by ceri and Adam Wolk
May 12 2021
Looks good to me, let's get this commited.
May 7 2021
I think it would be better to check for the individual component parts, as @ygy suggests, and if any of them aren't found, recommend installing ports/textproc/docproj - but only print the error message once, even if multiple dependencies aren't found.
May 5 2021
May 1 2021
Apr 28 2021
[source,shell] would indicate that a particular section using monospaced blocked text is a shell script.
Apr 24 2021
@dbaio Since it has been approved by a ports commiter (you), doceng (bcr), and a doc commiter (me), can I go ahead and commit this to the ports tree, or do you want to?
Apr 23 2021
I will note that PDF is mentioned as an optional part in the FDP quickstart, but I don't think it's going worth holding up the review over this.
Apr 22 2021
Apr 16 2021
Apr 12 2021
Apr 11 2021
Adding a bunch of spaces doesn't need much approval, but have some anyway. :)
Apr 10 2021
Only one tiny nit left, then you can go ahead and commit it with Approved by: debdrup
Looks good to me now, go ahead and commit with Approved by: debdrup
Apr 5 2021
Mar 31 2021
Mar 30 2021
Looks good to me. :)
Mar 26 2021
Mar 23 2021
Mar 22 2021
Mar 21 2021
Mar 20 2021
Looks good to me.
Mar 19 2021
Mar 18 2021
The patch doesn't apply cleanly for me, and it doesn't look like a trivial fix.
The only thing I'd note is that [source,bash] should be [source,shell].