On 04/16/2012 10:30 AM, April Chin wrote:
I'm looking for a code review for my updates to pylint, logilab-astng,
and logilab-common.
These three components must be updated together, since the updated
pylint has
a runtime dependency on the new versions of logilab-common &
logilab-astng;
there is no build dependency.
I'm still waiting for approval for the license change for
logilab-astng & logilab-common,
so the current license files may not be final.
Webrev:
http://jurassic.us.oracle.com/net/sfwcfi/builds/apchin/pylint-upgrade/webrev-1/
* If there isn't already an RFE in Bugster to add unittest2 to Solaris
(so we can properly test these components), then please file one.
.../components/logilab-astng/Makefile
line 23: need to update Copyright line to include 2012.
.../components/logilab-astng/logilab-astng-26.p5m
.../components/logilab-astng/logilab-astng-27.p5m
line 25: Is this transform correct? Are you wanting to apply the facet.devel
facet to all files under
usr/lib/python2.6/vendor-packages/logilab/astng/test ?
My .p5m regular expression-fu is sucky, but don't you need to
add a wildcard at the end? You could also add one at the beginning
to reduce the line length. Something like:
<transform dir file link hardlink path=usr.*astng/test/.+ -> default
facet.devel true>
Check with Danek or alanc or somebody else who totally groks this.
* Can you not add another transform to add "pkg.tmp.autopyc=false" in a
similar way, to all the files it applies too?
.../components/logilab-astng/logilab-astng.p5m:
line 43: I've noticed that in some other Python components under
.../componments/python/ in the Userland ws, where there is a
<component name>.p5m that has dependencies upon 2.6
and 2.7 packages, that there isn't a license line in the
<component>.p5m file. That suggests you don't need one here either
(but check with Norm).
.../components/logilab-common/logilab-common-26.p5m
.../components/logilab-common/logilab-common-27.p5m
line 25: similar concerns as to the logilab-astng .p5m files above.
* If you added "pkg.tmp.autopyc=false" to a variety of the test Python files
in the logilab-astng .p5m files, shouldn't they be added here too?
.../components/logilab-astng/logilab-astng.p5m
line 42: similar concern to line 43 in logilab-astng.p5m.
.../components/pylint/pylint-26.p5m
* Should these vendor-packages/pylint/test files have the facet.devel
facet too?
* Similar concerns w.r.t. "pkg.tmp.autopyc=false". Also, it seems to be on
some files (eg. .../test/input/func_continue_not_in_loop.py), but not
others (eg. .../test/input/func_class_members.py). What's the difference?
lines 415-418 and 420-423. I'm a little confused why these are commented
out now (rather than being removed like the comment at lines 412-414
suggests
should happen if pkgdepend is now fixed). If it's not fixed, why are they
now commented out?
.../components/pylint/pylint-27.p5m
* Should these vendor-packages/pylint/test files have the facet.devel facet?
* Similar concerns to pylint/pylint-26.p5m w.r.t. "pkg.tmp.autopyc=false".
.../components/pylint/pylint.p5m
* Similar concern to logilab-astng.p5m and logilab-astng.p5m on whether you
need a "licence ..." line in this .p5m file.
_______________________________________________
userland-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/userland-discuss
