Bart Smaalders wrote:
> http://cr.opensolaris.org/~barts/17581/
client.py:
- line 168: --exclude should be --reject. Also need a "\n" at the end"
- line 169: the alignment of the string needs to match that of previous
lines.
misc_non_gui.py:
- copyright update
pkg.1.txt:
- It might be worthwhile explaining -- either in the text describing
--reject or in an example -- what this would actually be used for.
- You need to reference --reject in the update section of the page.
api.py:
- Is "disallow_list" a holdover from a previous round of bikeshedding?
Shouldn't we just call this "reject_list" for consistency with the cli
flag? (I mostly don't care, but you've also got "removal_list" and
"disallow_dict" over in pkg_solver.py, and it's not clear if they
really should have the same name or not.)
- line 625: need a newline before this, and ditch the trailing quote.
api_errors.py:
- no need for a copyright update if there aren't any changes.
imageplan.py:
- line 1: Ditch this change.
pkg_solver.py:
- line 171: "removal_list" isn't in the argument list. You have
"disallow_dict", though, which presumably isn't a list.
- line 200, 538: match indent with line 195 ("removal_list = [")
t_pkg_install.py:
- line 677: --without should be --reject
- line 781: is there any way that you can coerce the solver to prefer B
over C in this case? Otherwise, we might just be getting lucky here.
- line 785: "NOR" -> "XOR"?
- line 789: why the stars here? If you need them, they should be
quoted.
- line 794: maybe also test that install --reject A A also fails sanely?
- line 801: you can simply save the contents of self.output, rather than
catting to and from a file.
- line 805, 810, 819: debugging?
- line 809: this just returns us to only no-idrs being on the system?
What is this testing -- just patterns?
- line 823: here's a place to test patterns:
update -v --reject idr\* ker...@...
- line 826: "reinsall" -> "reinstall"
- line 830: two spaces after list.
- Is there any part of the test here that really needs two builds of IDR
1?
You're also adding a bunch of trailing whitespace. The way I check for
this is to look at the patch in vim with the 'list' option set, and search
for ^+.*\s\+$. I'm not sure what to do for emacs to reduce creating them
in the first place.
Thanks,
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
