We should put "local" (without quotes) before each of these (non-blank) lines

We'll want a "local module" (w/o quotes) at the top of the function (first line after curly) to localize the variable used here in the for-loop

Let's reduce the indentation of these lines by a single tab

Let's remove this line -- it's not necessary unless using the dpv/cat pipe-through to redirect method

Since renaming docker_zfs_dir to docker_dir (reduction of 4 letters), is that enough to fit this all one one line that is 79 characters-or-less? (if no, retain line continuation)

It still doesn't fit in 79 cols :(

Initialization to NULL is required for all but module and for purposes of minimal diffs, prefer making these separate statements

[tab]local module
[tab]local fs_type=
[tab]local make_zfs=
[tab]local zfs_mountpoint=
[tab]local zfs_name=
[tab]local zpool_name=
[tab]local zpool_size=

Preferred for minimal diffs:

[tab]for module in \
[tab][tab]zfs \
[tab][tab]pf \
[tab][tab]linux \
[tab][tab]linux64 \
[tab]; do

Thanks for checking ;D

There should be a space after > and 2> but more importantly, this is preferred:

[tab][tab]pgrep -F /var/run/docker.pid > /dev/null 2>&1

Preferred:

[tab][tab]if pgrep -F /var/run/docker.pid > /dev/null 2>&1; then
[tab][tab][tab]echo "Docker already running? /var/run/docker.pid"
[tab][tab][tab]exit 1
[tab][tab]fi

You don't need a semicolon here if do is on the next line.

This isn't portable, but our sh supports it.

Why add the g here that forces you to bump PORTEPOCH ?
This is a bad idea, remove the g.

Use USE_GITHUB=yes.

default value, remove.

Remove this from the GH_TUPLE and add GH_SUBDIR=src/github.com/docker/docker instead.

This is going to become 17.05.0 before this patch lands, which will involve a PORTEPOCH bump.

Well, when this becomes 17.05.0, then PORTEPOCH will be needed, but right now, with a version of 20170907, there is no need to do it.

The reason why you should use:

[tab][tab]linux64 \
[tab]; do

Instead of:

[tab][tab]linux64
[tab]do

Is because list management (e.g., sorting with sort, moving items manually for ordered operation, etc.) is possible when all the elements have a trailing backslash. By making the last element not have a trailing backslash, a future diff that appends a new item to the end will not just be a diff with a single "+" line but instead a "-" and "+" pair where the backslash had to be added to the now-previous last-line in the list.

This also prevents mistakes.

What isn't portable? Space after the redirect operator? The duplication of fd1 to fd2 (to send stderr the same place as stdout)? Ordered file descriptor operations? All of those things are portable and no system I have touched in 20 years of using UNIX has ever balked at any of these. I would be shocked if you could find any variant of shell that balks at any of these (note: csh/tcsh/zsh don't count; things like ksh, dash, ash, do count -- it has to be a bourne shell variant, bash included since CentOS, Mac OS X, etc. use bash as /bin/sh).

Remove, default value.

The 80's called, they're telling me they had wider terminals back then and we have not had that limitations since before FreeBSD was born.

Religion called, they want their hate back.

The port is named docker-freebsd, so wouldn't that mean the default value would be backward? The GitHub account is freebsd-docker, not docker-freebsd.

Been thinking a lot that we should make the pidfile able to be set in rc.conf(5). E.g., docker_pidfile

$docker_pidfile instead of hard-coded value repeated throughout

$docker_pidfile instead of hard-coded value repeated throughout

$docker_pidfile instead of hard-coded value repeated throughout

Oh, yes, my brain rearranged the worlds in the same order, sorry about it.