- User Since
- Jan 12 2018, 3:33 PM (179 w, 5 h)
Fri, Jun 11
I don't really have time for this right now, so someone else can go ahead and commit it once it's been accepted.
Tue, Jun 1
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].
Mar 16 2021
I would also suggest adding a link to https://wiki.freebsd.org/IRC/Cloaks, as that explains the requirements, et cetera.
Once you've tested that it looks good, with make -C /usr/doc/documentation (run|build), you're welcome to go ahead and commit this. :)
Mar 15 2021
Can you find a commit hash for this changed behaviour?
Nevermind, just after I wrote that, I remembered something.
It's implemented using doc/shared/lib/ManPageMacro/extension.rb
I'm unsure about inlining commit hashes or revisions, so I think @gjb will have to weigh in on whether commit hashes should be referenced at the end like it's done with other things mentioned in the release notes.
I'm more partial to this, as this is how release notes have always looked, but I'm not against the change, as long as it's uniform across the whole document.
The author of the feature has added D29275 which was being worked on before this review started?
So it seems like this has been overcome by events.
I would also highly recommend adding some commit hashes.
If my memory serves, several of the commits involved have information that's relevant in configuring these new features.
Mar 14 2021
Mar 13 2021
Mar 9 2021
Looks good to me!
Mar 7 2021
I'll happily give approval for this from docs if it helps this commited.
It's much easier to remember one port than a handful (or having to look them up in the FDP).
Looks good to me.
Mar 6 2021
Looks good to me now, let's get this landed.
Mar 5 2021
I've rephrased it a bit, to avoid multiple sentences which rendered as separate lines when built.
Feb 27 2021
Feb 26 2021
More to the point, adding a few lines is not going to waste peoples diskspace as much as you think, as manual pages are compressed when installed.
My point is that if that's what this change does, it's not evident that that's what it does.
After talking it over on IRC, I think this needs little more descriptive information as to what the options in the config files do.
I was only going to remark on the gitrepo thing, but that's already had multiple layers of paint, so I shan't belabor the point.
Feb 25 2021
Feb 24 2021
Rework review to address outstanding issues with examples
It's possible Phabricator mangled things.
I forgot to accept this, didn't I.
Once these two minor things have been fixed (which can be done before commiting pushing), it looks good to me.
Feb 23 2021
Just one small thing, and as long as it builds (which it should, from the looks of it), it looks good to me.
This time actually correct the hash length, pointed out by @lwhsu