Paul, Thanks for your review. I completed most of the updates, but have a couple of questions in line.
New webreve posted at http://cr.opensolaris.org/~blong/logilab-astng/ incremental at: http://cr.opensolaris.org/~blong/logilab-astng.incr/ Paul Cunningham wrote on 05/07/09 02:46: > Bobbie, > > See comments below ... > > Paul > > Bobbie wrote: >> >> I am working on porting the logilab-astng python libraries. These >> libraries are used by pylint (python lint). If I could get some >> review and feeback, I'd appreciate it. >> >> http://cr.opensolaris.org/~blong/logilab-astng/ > > === Start of comments === > > 1. usr/src/Targetdirs > & usr/src/lib/Makefile > & usr/src/pkgdefs/Makefile > What is the stuff that webrev shows as deleted ? > Does it need resyncing with the gate. Sorry, I'd updated the webrev based upon another feedback and forgot to collapse the deltas. > > 2. usr/src/lib/logilab-astng/METADATA > The SRC: link didn't work when I tried it, please check: the > following works though .. > "http://ftp.logilab.org/pub/astng/logilab-astng-0.19.0.tar.gz"; > > The URL: link could be (I think) ... > "http://www.logilab.org/project/logilab-astng"; either works, so I changed it to your suggestion as it's more clear and logical. > > 3. usr/src/lib/logilab-astng/Makefile.sfw > Delete line ... > 36 $(VER)/build/proto: > and make that rules contents those of the 'all:' rule. done > > Line 37, change 'python' to '$(PYTHON)'. > > Add 'env - ' in front of all the 'python' invocations. done > > Lines ... > 43 (cd $(VER); $(PYTHON) setup.py install --root=$(LOCALPROTO)) > 44 (cd $(VER)/$(LOCALPROTO)/usr/lib/python2.4/site-packages; \ > 45 cp -R * $(REALPROTO)) > why not install it directly into the ws proto area? > Then use 'protofix' to correct the file permissions. done > > In rule ... > 9 $(VER)/setup.py > do you need to 'touch' the 'setup.py' file after > extracting the tarball I added this, but am Makefile illiterate. Can you explain why it's needed? everything seemed to work the same without it. > > After line 27 (VER=) add ... > TARBALL=$(VER).tar.gz > then replace '$(VER).tar.gz' with '$(TARBALL) throughout. done > > Line ... > 29 REALPROTO=$(ROOT)/usr/lib/python2.4/vendor-packages > a number of integrations are now using '/usr/lib/python2.4' so > you might want to define it in Makefile.master and use that, > for example something like ... > In Makefile.master define > PYTHONLIB=/usr/lib/python2.4 > in Makefile.sfw change line above to .. > REALPROTO=$(ROOT)/$(PYTHONLIB)/vendor-packages Back to my being Makefile illiterate, I'm reluctant to change the master due to any potential impact to the other packages. We're already going to have some conflict resolution during the merge, so I was hoping to diminish this. Or is this simply ignorance on my part, and it would improve things for everyone using the 2.4/vendor-packages? > > 4. usr/src/lib/logilab-astng/sunman-stability > Is this used, if not delete it removed > > 5. usr/src/pkgdefs/SUNWlogilab-astng/prototype_com > Remove the write permission bits from the files installed > into /usr. And in Makefile.sfw use 'protofix' to correct them > in the proto area (see item (3)). done > > === End of comments =====
