Danek Duvall wrote:
On Wed, Mar 25, 2009 at 04:43:33PM -0600, Mark J. Nelson wrote:
http://cr.opensolaris.org/~mjnelson/webrev.its.3/
All of these are nits:
webrev.sh:
- line 2673: Is there a particular reason that the conf files are
cumulative while the registry files aren't? It seems to me that it'd
be nice to add to the registry without having to duplicate it.
Yes. I want people to add systems to the registry, not bypass it. The
only time I really want people overriding the registry is when they're
testing an update to it prior to integration.
The conf files are cumulative to allow overriding a subset of the settings.
- line 2688: While there's no real danger here, eval always scares me.
Since you only anticipate two variables here, you could do the same
thing you do for the registry file, and simply set the two variable
explicitly, based on the names that are read in.
True. But I really like that the conf file is extensible this way, even
to the point of setting your default to "-O" by adding "Oflag=1" there.
I'll change it if you or others think it's too big of a security hole.
- line 2725: Why define an EXTERNAL_URL in the registry when you could
just define URL, and then you wouldn't need this block?
The initial pass was "URL" and "SWAN_URL," and when I generalized
"SWAN_URL" to be "INTERNAL_URL_domain," it seemed a natural change.
"Natural," that is, in the context of the its.reg file, which is where I
expect people to care. Clearly the webrev.sh processing is slightly
different, though I could keep EXTERNAL_URL and still skip this part by
using ${itsinfo["${p}_URL"]:?${itsinfo["${p}_EXTERNAL_URL"]}} later on
when I translate to a hyperlink.
What do you recommend?
--Mark
_______________________________________________
tools-discuss mailing list
tools-discuss@opensolaris.org
