Skip to main content

[pkg-discuss] Re: Code review request: 15678637 image-create and set-publisher only use last -p specified.

  • From: Alta Elstad < >
  • To:
  • Subject: [pkg-discuss] Re: Code review request: 15678637 image-create and set-publisher only use last -p specified.
  • Date: Wed, 31 Jul 2013 10:28:08 -0700

On 07/31/13 09:55, Shawn Walker wrote:
On 07/31/13 08:38, Erik Trauschke wrote:

On 07/31/13 12:02 AM, Thejaswini wrote:
Hi

The webrevhttp://ips.java.net/webrev/tk241774/15678637/rev01/
includes fix for:

15678637 <https://bug.oraclecorp.com/show?15678637> SUNBT7133912
image-create and set-publisher only use last -p specified.

The change is trivial.
The fix is :
To exit pkg with a error message when user provides -p option >1 in
"pkg image-create" and "pkg set-publisher" command.

Please review the changes and let me know your comments.

client.py:

Maybe rephrase to: "-p option can only be specified once"?

Yes; I'd ask Alta about the wording. It should be similar to what we'd use in 
a
man page.

Currently the man page has no prose statement about how many times -p can be specified, either to set-publisher or image-create, as I'm sure you are aware. Currently this is handled by not including ... in the syntax. Are you suggesting such a statement is needed in the man page?

In the usage message, I think the above is fine, though I prefer complete sentences if appropriate for a usage message: "The -p option can be specified only one time."

t_pkg_image_create.py:

751, 752: I think it's not really necessary to actually create these
directories because you are just testing if the option-parsing code
fails. These directories are never evaluated.

Correct. No need to call mkdir().

The copyright year needs updating.

Also, the test logic should be moved into test_image_create_bad_opts(); it
doesn't need it's own unit test. You also don't need to check the error 
output.
Checking for exit=2 is sufficient.

-Shawn


[pkg-discuss] Code review request: 15678637 image-create and set-publisher only use last -p specified.

Thejaswini 07/31/2013

[pkg-discuss] Re: Code review request: 15678637 image-create and set-publisher only use last -p specified.

Erik Trauschke 07/31/2013

[pkg-discuss] Re: Code review request: 15678637 image-create and set-publisher only use last -p specified.

Shawn Walker 07/31/2013

[pkg-discuss] Re: Code review request: 15678637 image-create and set-publisher only use last -p specified.

Alta Elstad 07/31/2013

[pkg-discuss] Re: Code review request: 15678637 image-create and set-publisher only use last -p specified.

Shawn Walker 07/31/2013

[pkg-discuss] Re: Code review request: 15678637 image-create and set-publisher only use last -p specified.

Erik Trauschke 07/31/2013
 
 
Close
loading
Please Confirm
Close