Dne  1.12.11 16:27, Isaac Rozenfeld napsal(a):
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?

Hi Isaac,
thanks for your suggestion. Adding spaces around / should be easy, however it adds 2 more chars.

This CR doesn't deliver an update to the man page but man page should be updated. What do you suggest ? Should I modify man page myself or should I file documentation bug (which category) ?

I would change:

         -m|--manifest

             Optional: Lists the manifests and scripts associated
             with the install services on a local server.

             When -n is not specified,  displays  an  abbreviated
             listing per service. This includes the default mani-
             fest or script, and all  non-default  manifests  and
             scripts  that  have  criteria  associated with them.
             Criteria are not listed.

             When -n is specified,  displays  all  manifests  and
             scripts for the given service, using a more complete
             listing format that includes criteria for each mani-
             fest.  Inactive  manifests, which have no associated
             criteria and are not designated as the default,  are
             so  marked. Criteria associated with a default mani-
             fest are marked as inactive.

to:
         -m|--manifest

             Optional: Lists the manifests and scripts associated
             with the install services on a local server. Listing
             includes criteria for each manifest. Inactive  mani-
             fests, which have no associated criteria and are not
             designated as the default,  are so  marked. Criteria
             associated with a default manifest are marked as ignored.

             When -n is not specified, displays all manifests and
             scripts for all services.

             When -n is specified,  displays  all  manifests  and
             scripts for the given service.


However because English is not my native language there might be grammar or stylistic mistakes.

Best regards,

Tomas D.


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

Reply via email to