Hello Tomas, Just a minor suggestion about the header: if we use slashes and 'Name' too closely, it could be confusing for newcomers. Want to make it more evident that services and manifests are different entities, but have a relationship.
Since a service always has at least one manifest, and the use of the 2nd line is the direction chosen, may suggest a header that illustrates the relationship: Service's Manifest(s) Btw, does this CR deliver an update to the man page, too? Thanks and kind regards, Isaac On Dec 1, 2011, at 8:44 AM, Tomas Dzik <[email protected]> wrote: > Hi Ethan and Dave, > I implemented your suggestion and repeated the tests. There will always be > cases when line will be longer then 80 chars because service name and > manifest name can be arbitrarily long. I was also thinking about not printing > Ignored before each criteria and printing it only once. On the other hand I > don't think that a lot of people will use ignored criteria. > > My new output is in attachment. > > New webrev is here: > > https://cr.opensolaris.org/action/browse/caiman/t.dzik/7036455-1/ > > Also I noted that (even in the original code) network is printed ugly > (without dots). > > Should I file separate bug and fix it ? (It seems to be in different part of > code then I am fixing now.) > > Regards, > > Tomas D. > > > Dne 30.11.11 22:58, Dave Miner napsal(a): >> Ethan, I think that's an improvement. I think we could also put some >> effort into minimizing the whitespace in the criteria column to improve >> readability. Might also consider whether Status could be compressed to >> just D, I, <blank> (there aren't any other states at this point, right?). >> >> Dave >> >> On 11/29/11 15:56, Ethan Quach wrote: >>> Tomas, Dave, >>> >>> One suggestion is to merge the Service and Manifest name columns into >>> one by outputting the service name in it's own line, and using a static >>> 3-space indentation for each manifest entry that belongs to that service >>> beneath it. This removes the length of the Service name from >>> contributing to the overall width. Your new (e) example would look like: >>> >>> root@S11:~# installadm list -m >>> >>> Service/Manifest Name Status Criteria >>> --------------------- ------ -------- >>> default-i386 >>> orig_default Default (Ignored: ipv4 = 129.168.1.2) >>> (Ignored: mem = 2048 MB - >>> unbounded) >>> default-sparc >>> orig_default Default None >>> >>> new_inactive_manifest Inactive None >>> >>> solaris11-i386 >>> orig_default arch = i86pc >>> mac = AA:BB:CC:DD:EE:FF >>> ipv4 = 10.0.2.100 - >>> 10.0.2.199 >>> cpu = i386 >>> mem = 1024 MB >>> network = 10000000000 >>> platform = i86pc >>> zonename = some-zone-name-1 >>> >>> some-new_manifest-1 arch = sparc >>> mac = AA:BB:CC:DD:EE:EE >>> mem = 1024 MB >>> >>> some-new_manifest-2bbbbbbb Default None >>> >>> solaris11-sparc orig_default Default None >>> >>> >>> >>> We used this approach for "beadm list [-d] [-s]". >>> >>> >>> -ethan >>> >>> >>> On 11/29/11 01:15, Tomas Dzik wrote: >>>> Hi Dave, >>>> thanks a lot for your comment. So taking into account that criteria >>>> themselves could be quite long (only mac=AA:BB:CC:DD:EE:EE without >>>> spaces around '=' occupy 21 characters) do you (or anyone else on this >>>> mailing list) have some suggestion how the output should look like ? >>>> >>>> Best regards, >>>> >>>> Tomas D. >>>> >>>> Dne 28.11.11 16:41, Dave Miner napsal(a): >>>>> Tomas, thanks for taking this on, it's an important improvement. But, >>>>> can we work out an output format that will keep the width at 80 columns >>>>> more often than not? The examples seem to be wider than that in >>>>> general, >>>>> and I'd expect complaints about readability as shown. >>>>> >>>>> Dave >>>>> >>>>> On 11/24/11 09:36, Tomas Dzik wrote: >>>>>> Hi all, >>>>>> I would like to ask you for a code-review for: >>>>>> >>>>>> 7036455 "installadm list -m" should show criteria >>>>>> >>>>>> Webrev: >>>>>> https://cr.opensolaris.org/action/browse/caiman/t.dzik/7036455 >>>>>> >>>>>> How it works: >>>>>> >>>>>> a) It prints for "installadm list -m" similar output like for >>>>>> "installadm list -m -n svcname" >>>>>> >>>>>> b) For each service it prints active manifests + their associated >>>>>> criteria >>>>>> fist, followed by default manifest (+ plus it's associated criteria >>>>>> marked as Ignored), >>>>>> followed by inactive manifests. In each of that categories order of >>>>>> manifest is not >>>>>> explicitly set >>>>>> >>>>>> c) For each manifest criteria are printed in given order. Order is >>>>>> exactly the same like >>>>>> if you run "installadm list -m -n svcname" >>>>>> >>>>>> d) For examples of output, please look at "Testing" paragraph. >>>>>> >>>>>> Testing: >>>>>> 1) I run all unit tests using ./slim_test -c tests.nose and there were >>>>>> no regressions >>>>>> 2) I run pep8 on file which I modified and there were no issues >>>>>> 3) I created 2 x86 virtual machines and I installed my built workspace >>>>>> on one of them >>>>>> and let the second-one for reference. Then I created several services >>>>>> with different manifests >>>>>> using different criteria. Here are the original outputs of installadm >>>>>> list compared to outputs >>>>>> after my fix: >>>>>> >>>>>> >>>>>> >>>>>> a) Original: >>>>>> >>>>>> root@S11:~# installadm list -m >>>>>> >>>>>> Service Name Manifest Status >>>>>> ------------ -------- ------ >>>>>> default-i386 orig_default Default >>>>>> default-sparc orig_default Default >>>>>> solaris11-i386 orig_default Default >>>>>> solaris11-sparc orig_default Default >>>>>> >>>>>> a) New: >>>>>> >>>>>> root@S11:~# installadm list -m >>>>>> >>>>>> Service Name Manifest Status Criteria >>>>>> ------------ -------- ------ -------- >>>>>> default-i386 orig_default Default None >>>>>> >>>>>> default-sparc orig_default Default None >>>>>> >>>>>> solaris11-i386 orig_default Default None >>>>>> >>>>>> solaris11-sparc orig_default Default None >>>>>> >>>>>> b) Original: >>>>>> >>>>>> root@S11:~# installadm list -m -n default-i386 >>>>>> >>>>>> Manifest Status Criteria >>>>>> -------- ------ -------- >>>>>> orig_default Default None >>>>>> >>>>>> >>>>>> b) New: >>>>>> >>>>>> root@S11:~# installadm list -m -n default-i386 >>>>>> >>>>>> Manifest Status Criteria >>>>>> -------- ------ -------- >>>>>> orig_default Default None >>>>>> >>>>>> c) Original: >>>>>> >>>>>> root@S11:~# installadm list -m >>>>>> >>>>>> Service Name Manifest Status >>>>>> ------------ -------- ------ >>>>>> default-i386 orig_default Default >>>>>> default-sparc orig_default Default >>>>>> solaris11-i386 orig_default Default >>>>>> solaris11-sparc orig_default Default >>>>>> >>>>>> c) New: >>>>>> >>>>>> root@S11:~# installadm list -m >>>>>> >>>>>> Service Name Manifest Status Criteria >>>>>> ------------ -------- ------ -------- >>>>>> default-i386 orig_default Default (Ignored: ipv4 = 192.168.1.2) >>>>>> (Ignored: mem = 2048 MB >>>>>> - unbounded) >>>>>> >>>>>> default-sparc orig_default Default None >>>>>> >>>>>> solaris11-i386 orig_default Default None >>>>>> >>>>>> solaris11-sparc orig_default Default None >>>>>> >>>>>> d) Original: >>>>>> >>>>>> root@S11:~# installadm list -m >>>>>> >>>>>> Service Name Manifest Status >>>>>> ------------ -------- ------ >>>>>> default-i386 orig_default Default >>>>>> default-sparc orig_default Default >>>>>> solaris11-i386 some-new_manifest-2bbbbbbb Default >>>>>> solaris11-sparc orig_default Default >>>>>> >>>>>> d) New: >>>>>> >>>>>> root@S11:~# installadm list -m >>>>>> >>>>>> Service Name Manifest Status Criteria >>>>>> ------------ -------- ------ -------- >>>>>> default-i386 orig_default Default (Ignored: ipv4 >>>>>> = 129.168.1.2) >>>>>> (Ignored: mem >>>>>> = 2048 MB - unbounded) >>>>>> >>>>>> default-sparc orig_default Default None >>>>>> >>>>>> solaris11-i386 some-new_manifest-2bbbbbbb Default None >>>>>> >>>>>> orig_default Inactive None >>>>>> >>>>>> some-new_manifest-1 Inactive None >>>>>> >>>>>> solaris11-sparc orig_default Default None >>>>>> >>>>>> e) Original: >>>>>> >>>>>> oot@S11:~# installadm list -m >>>>>> >>>>>> Service Name Manifest Status >>>>>> ------------ -------- ------ >>>>>> default-i386 orig_default Default >>>>>> default-sparc orig_default Default >>>>>> solaris11-i386 orig_default >>>>>> some-new_manifest-1 >>>>>> some-new_manifest-2bbbbbbb Default >>>>>> solaris11-sparc orig_default Default >>>>>> >>>>>> e) New: >>>>>> >>>>>> root@S11:~# installadm list -m >>>>>> >>>>>> Service Name Manifest Status Criteria >>>>>> ------------ -------- ------ -------- >>>>>> default-i386 orig_default Default (Ignored: ipv4 >>>>>> = 129.168.1.2) >>>>>> (Ignored: mem >>>>>> = 2048 MB - unbounded) >>>>>> >>>>>> default-sparc orig_default Default None >>>>>> >>>>>> new_inactive_manifest Inactive None >>>>>> >>>>>> solaris11-i386 orig_default arch = i86pc >>>>>> mac = >>>>>> AA:BB:CC:DD:EE:FF >>>>>> ipv4 = >>>>>> 10.0.2.100 - 10.0.2.199 >>>>>> cpu = i386 >>>>>> mem = 1024 MB >>>>>> network = >>>>>> 10000000000 >>>>>> platform = i86pc >>>>>> zonename = >>>>>> some-zone-name-1 >>>>>> >>>>>> some-new_manifest-1 arch = sparc >>>>>> mac = >>>>>> AA:BB:CC:DD:EE:EE >>>>>> mem = 1024 MB >>>>>> >>>>>> some-new_manifest-2bbbbbbb Default None >>>>>> >>>>>> solaris11-sparc orig_default Default None >>>>>> >>>>>> 4) Because I modified function which is also used for printing >>>>>> profiles >>>>>> I checked >>>>>> that listing of profiles didn't change >>>>>> >>>>>> Regards, >>>>>> >>>>>> Tomas D. >>>>>> _______________________________________________ >>>>>> caiman-discuss mailing list >>>>>> [email protected] >>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>>> >>>> >>>> _______________________________________________ >>>> caiman-discuss mailing list >>>> [email protected] >>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>> _______________________________________________ >>> caiman-discuss mailing list >>> [email protected] >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> >> _______________________________________________ >> caiman-discuss mailing list >> [email protected] >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > > <notes-7036455.txt> > _______________________________________________ > caiman-discuss mailing list > [email protected] > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss _______________________________________________ caiman-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
