On 06/10/11 00:13, Brock Pytlik wrote:
Webrev:
http://cr.opensolaris.org/~bpytlik/ips-18441-v1/
Bug:
18441 pkg needs a freeze mechanism
These changes provide the freeze mechanism I outlined previously. The
biggest changes I know of from the original use cases I sent out are that:
1) If pkg unfreeze <blah> doesn't match any frozen packages, no error is
reported. I decided that seems reasonable since pkg unfreeze details
each package that was unfrozen, so no output means no packages were
unfrozen.
2) Publishers can be part of the patterns used for matching, but a
package is only frozen to a version, not to a publisher.
I think that covers the major changes.
src/client.py:
line 2159: docstring indicates this only sets freezes, but it also
displays them
lines 2199-2200: Having unfreeze display packages as well as freeze
is inconsistent with the paradigm we use for other commands. I'd
rather not have both of these commands be capable of displaying the
list of frozen packages. This is also not documented in the man
page.
line 5366: what, not "thaw" instead? (joking)
src/man/pkg.1.txt:
lines 640-641: s/end up/be updated to/ ?
lines 648-649: suggest:
With -n, perform a trial run of the operation, displaying the
list of packages that would be affected by the freeze without
freezing any packages.
At least, I think it should display the list of packages it would
constrain. (And it seems that it does currently.)
You may also want to mention that publisher information in the FMRI
will be ignored. (That is, you can't place publisher-specific
freezes, although that might be a nice future RFE to allow
per-package stickiness.)
line 653: s/. Any/; any/
lines 655-656: Suggest:
With -n, perform a trial run of the operation, displaying the
list of packages that will be unfrozen without unfreezing any
packages.
Like above, I'd hope it would display the list of what would be
affected. (And it seems that it does currently.)
No examples?
src/modules/client/api.py:
line 615: Can you rename 'message' to 'comment' or 'reason' to be a
bit more descriptive? And if so, change it everywhere else?
'reason' would at least be consistent with the man page.
lines 617-619: copy paste? (still refers to avoid/unavoid)
src/modules/client/api_errors.py:
line 2740: s/were/are/ ?
line 2754: s/don't contain/contain no/ (to avoid the double don't)
src/modules/client/image.py:
line 3725: s/users/user's/
lines 4061, 4071: s/pacakges/packages/
src/modules/client/imageplan.py:
lines 2323: The variable names are a bit hard to grok, can you rename:
'not_match_ok' -> 'raise_unmatched' and 'not_inst_ok' to
'raise_not_installed'? That would also have the benefit of getting
rid of the double negative expressions on lines 2454-2455 (e.g.
not not_match_ok).
line 2515: same concern as that on 2323 for variable name, thanks
line 3003: brief docstring appreciated
line 3050: s/==/ == /
line 3060: 4-space continuation indent
src/modules/client/pkg_solver.py:
lines 369-371, 663-664: suggest (mainly to get rid of freeze's reason):
This version is excluded by freeze {0}. The reason for the freeze is: {1}
src/tests/cli/t_pkg_freeze.py:
line 38: shouldn't this be True? (image are destroyed and created
every time anyway I thought...)
Missing a test to verify bad FMRI input doesn't cause traceback.
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
