David Bustos wrote:
> Quoth Alan Maguire on Wed, Jan 09, 2008 at 04:34:11PM +0000:
>   
>> thanks Liane (and Tom!) for taking a look. i've refreshed
>> the webrev, so let me know if there's anything else amiss.
>>
>> http://cr.opensolaris.org/~amaguire/svccfg_refresh/
>>     
>
> cmd/svc/svccfg/svccfg_help.c
>   124: Since the snapshot is named "running", please don't capitalize
>     its name here.
>
>   
ok.
>   124: The other help text uses "currently selected instance", so please
>     follow suit here.
>
>   
sure.
>   124: "Update the Running snapshot..." is technically correct, but
>     I suspect we can make the purpose more apparent to users with
>     something like "Commit any changes to the directly attached
>     properties to running snapshot."  Note that the "directly attached
>     properties" language is from the svcprop manpage.  I'm not sure
>     whether we should also use "effective properties" from there, too.
>     The svcadm manpage also uses "running configuration snapshot", which
>     I am not sure is worth the extra terminal space here.
>
>   125: As in the description of refresh on svcadm(1M), we don't know
>     that the service's restarter acknowledges refresh methods.  So
>     I suspect we should change this to something like "If the repository
>     is being monitored, the service's restarter will be informed of the
>     change.  If the restarter is svc.startd(1M), the refresh method will
>     be invoked."  Though that seems like an unnatural combination of
>     genericity and specificity, for some reason.
>
>   
hmm, how about something like:

"For the currently selected instance, update the running configuration
snapshot with the values from the current configuration.  If the currently
selected repository is being monitored, the service's restarter will be
informed of the change, and if this restarter is svc.startd(1M), the refresh
method will be invoked (if it exists) to request the service reread its own
configuration.  For other restarters, see the restarter documentation."

it's based mostly on the language in "svcadm refresh", combined with
most of your suggestions. i'm a bit wary of introducing new words
like "commit" though, as they might suggest there's a new or different
concept in play with the svccfg refresh subcommand. let me know what
you think.
> cmd/svc/svccfg/svccfg_libscf.c
>   4986: Please note that snap is used as scratch space.
>
>   12369,378,399,408: I think the colon obviates the parentheses.  But
>     I believe the rest of svccfg uses parentheses without the colon.
>
>   12384,419: I don't think you should free(fmribuf) before bad_error()
>     so that fmribuf will be in the corefile.
>
>   
all accepted. i'll respin once we finalize the help text. thanks!

alan

Reply via email to