Oh, I missed that there was a new review a couple years ago. I am more than happy to dust this off. I was just talking about this to a coworker last night at the company holiday party.

Fix variable name in awk -- s/time/tm/

Advertise -j option in stats usage statement

Bump copyright

Update

Full context as-requested by emaste

In D42242#963650, @emaste wrote:

FYI for future Phabricator uploads please include full context, e.g. git show -U99999 <hash>. Details are at https://wiki.freebsd.org/action/show/Phabricator

LGTM

Bump port to 1.2 so we catch latest development (the inclusion of a dashboard for the Block I/O stats)

Fix distinfo

Update diff before applying to head

I was working on committing this and went to go apply the patch to the tree and the patch no longer applies cleanly.

Add missing dashboard examples from 1.1

Fixup package COMMENT to match what I have set for 1.1

Pepper [@]sample

In D36051#824457, @woodsb02 wrote:

You’re probably already aware, but ports has a commit hook so that commit messages should start the first line with “category/portname: description”.

So commit message should have first line:
sysutils/dwatch-json: update to 1.1, restore maintainership

Reclaim

Tested and ready for re-review

Add missing io-sample conf

makesum for 1.1

Add symbolic link entries from 1.1

Add missing config to plist for 1.1

Update pkg-descr for 1.1

Update the plist for 1.1

Abandon the update to 0.5 and update to 1.1

Great work. I gave it a thorough check, inside and out. Green-lit to go.

From my 2018 BSDCAN presentation slides, here are the modules in base ... we're not going to add "dwatch-" prefix to all those simply because a PORT convention says that a PORT should install files named the same as the PORT when the PORT should follow BASE convention already set forth 6 years ago

In D36051#818573, @diizzy wrote:

I discussed this submission with dteske on IRC and possibly have a few suggestions:

PORTNAME should correspond to the naming convention of installed files and usecase whenever possible, so dwatch-json --> dwatch-json-net and preferably libexec/dwatch/json-net* --> libexec/dwatch/dwatch-json-net*

Grafnet should be broken out, put in a separate port and be set as a dependency (if required) as it doesn't seem to be a hard requirement and/or limited to dwatch-json-net looking at pkg-descr

I have two suggestions, but you're only meant to pick one. I don't care which. Minor optimization or major optimization.

Sorry for being late to the party

There we go. This looks to be all good.

The raw diff [1] looks funny. The "before" (line removed) doesn't match what is in head [2].

In D35357#801773, @brd wrote:

In D35357#801606, @dteske wrote:

Can we discuss perhaps using K&R style naming?

That is to say, BSDINSTALL_SKIP_* instead of BSDINSTALL_*_SKIP -- it would go a long way to being able to more-easily grep-out the skippable options and also open the prefix to being used in other parts of bsdinstall that are unrelated (read: other scripts/functions)

I was thinking in a follow up commit about adding more variables, BSDINSTALL_TIME_NAME and BSDINSTALL_TIME_DESC to populate the menu in finalconfig(). Doing it that way seemed a bit nicer to me, but I don't feel strongly about it. So instead we could use BSDINSTALL_SKIP_*, BSDINSTALL_NAME_* and BSDINSTALL_DESC_*.

Can we discuss perhaps using K&R style naming?

I will also note that the removal of the "export" may cause issues. There would be other acceptable solutions, such as:

Changes required.

In D35331#801282, @asomers wrote:

Oh, sorry about that. I'll commit the fix. I didn't even realize that it was possible to leak memory in sh.

Fix a memory leak introduced by previous commit.