2008/6/27 Tom Mueller (pkg-discuss) <[EMAIL PROTECTED]>: > Please review the changes for the following feature: > > 856 Image properties > > webrev: http://cr.opensolaris.org/~tmueller/cr-856/
http://cr.opensolaris.org/~tmueller/cr-856/doc/image.txt.wdiff.html ========== 108 + /name name of image for use in GUIs and other UIs The right column text doesn't seem to line up with other text indented similarly on lines 114, etc. Line 113 seems to be off by one as well though you didn't change it. http://cr.opensolaris.org/~tmueller/cr-856/src/client.py.wdiff.html ========== 1303 + """pkg property""" Shouldn't this be "pkg property [propname ...]" ? http://cr.opensolaris.org/~tmueller/cr-856/src/man/pkg.1.txt.wdiff.html ========== 221 + names and values for all image properties. If a specific list of Two space needed before "If a specific...". 223 + properties. With -H, omit the headers from the listing. I just noticed that we're inconsistent about how we summarise what the various options do for a particular subcommand. As an example, on lines 115-122, we list each option on its own line. The same on lines 135-142. However, in other places, we describe how the option affects the output in the text describing the subcommand itself. Which way do we want to to this? I'm just seeking consistency. http://cr.opensolaris.org/~tmueller/cr-856/src/man/pkg.5.txt.wdiff.html ========== 344 + Properties 345 + Each image can be associated with properties using pkg(1). Image 346 + properties may be used by higher level tools and users to store information 347 + about the purpose and content of the image. Can I suggest alternate wording? Properties Images can have one more properties associated with them using pkg(1). These properties can be used to store information about the purpose and content of the image. http://cr.opensolaris.org/~tmueller/cr-856/src/modules/client/imageconfig.py.wdiff.html ========== 132 139 cp.add_section("policy") 133 140 for p in self.policies: 134 141 cp.set("policy", p, str(self.policies[p])) 135 142 143 + cp.add_section("property") 144 + for p in self.properties: 145 + cp.set("property", p, str(self.properties[p])) 146 + 136 147 cp.add_section("filter") 137 148 for f in self.filters: 138 149 cp.set("filter", f, str(self.filters[f])) It looks like this could be condensed to something like (haven't tried this): for sname in ["policy", "property", "filter"]: cp.add_section(sname) attrs = self.getattr(self, sname) for p in attrs: cp.set(sname, p, str(attrs[p]) http://cr.opensolaris.org/~tmueller/cr-856/src/tests/cli/t_commandline.py.wdiff.html ========== What about a test to see if -H is accepted and honoured for the property subcommand? Cheers, -- Shawn Walker _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
