https://cr.opensolaris.org/action/browse/pkg/timf/periodic-recv-3/
src/svc/pkg-repositories-setup.xml
Does this service really need dependencies on
system/filesystem/local and system/filesystem/autofs given that
the dataset must be local?
Sorry, copy/paste error for autofs :-/ I think it needs filesystem/local
because I'd like any other mounts near /var/share to appear before I try to
create this new dataset.
Yes, sorry - I meant milestone/network and system/filesystem/autofs. I
don't believe the former needs to be listed as a dependency either.
src/svc/pkg5_include.sh
Lines 101-109 - The window is very small already but I suppose
if you wanted to reduce it further, you could say:
$DIFF /tmp/actual-crontab.$$ -< $CURRENT_CRONTAB 2>&1 \
> /dev/null
if [ $? == 0 ]; then
$CRONTAB $NEW_CRONTAB
EXIT=$?
else
echo "Crontab file was modified unexpectedly!"
EXIT=1
fi
$RM /tmp/actual-crontab.$$
return $?
Sounds good, though 'return $EXIT' as the last line, I think.
Yes, that last line should be 'return $EXIT'.
src/svc/svc-pkg-mirror
Line 206 - Is the "exit 1" actually necessary here as we should
have never returned from check_failure?
It's required because I'm using the argument 'degrade' on this call to
check_failure(), which causes it to mark the service as being in maintenance,
but returns to allow us to continue cleaning up.
Yup, got it.
Oh, I didn't know about that (though I think you mean -q? but yep)
Yes, -q for grep(1). I typically use egrep(1) where the the equivalent
option is -s.
src/svc/pkg-repositories-setup.xml
Lines 47-56 - As above, I believe this dependency is
superfluous.
src/svc/pkg5_include.sh
Line 118 - As above, should be 'return $EXIT'.
Line 143-144 - Nit, multiline comment not in Shapiro normal
form.
src/svc/svc-pkg-mirror
Lines 99-100, 122-124, 143-145 - Nit, a few more multiline
comments not in Shapiro normal form.
Line 372 - ']&& [' should be '] && ['.
I think you can also collapse some of the lines together on the
echo.
src/svc/svc-pkg-repositories-setup
Line 62 - Not sure what I was thinking previously. The old code
where you saved the exist status from the "$ZFS allow" before
calling $CHOWN was better. That said, perhaps it would be
clearer to avoid the default setting of
result=$SMF_EXIT_ERR_FATAL and instead explicitly 'exit
$SMF_EXIT_ERR_FATAL' in the cases where the $ZFS create, allow
or $CHOWN fail (with appropriate messaging?)
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
