Hi Paul, thanks a lot for your review!
On 13/11/2011 20:25, Paul Tagliamonte wrote: > INADD, but here's a review, > > General note: > - You've got a lot of stuff in ./debian/ - I would invite you to > consider creating a few directories to put files that relate to > eachother (theme related stuff, and so forth) I took the wmaker package from the previous maintainer, and I try to update it to the current standard-version, dep-3/5,... Some things are adopted from the previous package. I have a lot of files in ./debian because this package creates nine or ten binary packages. The files are *.install, *.symbols, *.docs,... But probably yes, I can move/delete some files: delete: TODO, nightly_build.sh move: WMWindowAttributes, debian.tiff.uu, appearance.menu-method, Debian.theme > You're overriding a lot of stuff. Can you please explain them all? > Some of them look valid... First, I don't know how to solve these problems. I try to use "http://www.debian.org/doc/packaging-manuals/menu.html/"; to create/update the windowmaker menu's. But Window Maker is an exception and is not too easy to create the menus. In http://www.debian.org/doc/packaging-manuals/menu.html/ch3.html you can find: Window Maker This section is reserved for wmaker as a special case. All wmaker specific entries must go here. For this reason we have these overrides. I try to change some things, but then the WindowMaker menu system fails :-( > You should consider putting a comment block by each override, and note > exactly why. If lintian's in error, please report a bug (and don't > override!) I think a little on this. IMO wmaker is an exception, and probably is easy to continue with the overrides and don't modify lintian. > wmaker: menu-item-creates-new-root-section > wmaker: menu-command-not-in-package > wmaker: menu-item-contains-unknown-tag > wmaker: menu-item-needs-tag-has-unknown-value > > ./debian/debian.tiff.uu > - You can avoid uuencoding with source format 3 (as you're using) I will try to find more info about it. > ./debian/nightly_build.sh > - what is this, and why do we need it in the archive? :) Yes, this file is used in the webpage to create a nighty build :-). Of course should be removed from the Debian package. > ./debian/TODO > - Consider keeping track of this in the BTS Should be removed too. The TODO info don't apply now. > ./debian/copyright > You say: > License: public-domain > > but then you go on to say: > > They may be distributed freely and/or modified as long as the original > Author is mentioned! > > That's not public domain :) > > Also: > License: public-domain > do What The Fuck you want to Public License WTF license!! ;-) I try to select one license, but I found nothing. http://dep.debian.net/deps/dep5/ What should I do? > Lastly: > you have a GPL-2 block without files. No. Is the License paragraph. Is not correct? > ./debian/rules > I've never seen such a large dh-short rules file before. I can write a book about it. It took a lot of time. Was hard. Probably is a good idea to review some parts. > Consider some of the following: > > XLOCALE := --disable-locale > [snip] > WMAKER_OPTIONS := $(XLOCALE) $(MODELOCK) $(XINERAMA) \ > [snip] > > You're going to have to modify this file anyway, why not keep a > well-formatted list of the args directly. The package, in the previous version, had this info. Are "possible" options, then we select only a few of them: XINERAMA := --enable-xinerama # USERMENU := --enable-usermenu XINERAMA is used, USERMENU is not used, but is an option... I don't have preferences about it. If you think is better put the args directly, perfect. I know that I will send this package some times to mentors :-( It has many many thinks. > There's more, but I think it just needs a good hard think on what > really needs to be in there. > I mean, it's huge. > > I didn't get a chance to get too much farther then this, I just ran > out of laptop battery. Someone else can take over - it looks like this > needs a good stern clean. Thanks a lot for your time. Your comments are very good. Probably a second opinion should be nice, because this package is a little bit "rare" > Other then that, it's in fairly nice shape. Well done on maintaining > such a huge package. Thanks, your comments are very welcome! > Overrides aside, lintian reports one source pedant: I don't remember, but if I remove the quilt from the build-depends, I got an error about quilt dependence. But I am not sure. > > P: wmaker source: unneeded-build-dep-on-quilt > N: > N: This package build-depends on quilt, which is not required since > N: dpkg-source will apply patches at unpack time for 3.0 (quilt) source > N: packages. > N: > N: Remember to remove any references to quilt in the rules file (e.g. > N: "--with quilt", dh_quilt_* or quilt makefile includes). > N: > N: This tag is meant to remind people that with 3.0 (quilt), quilt is not > N: necessary. If you keep the build-depends on quilt to ease backporting to > N: older releases, then please ignore/override this tag. > N: > N: Severity: pedantic, Certainty: possible > N: > N: Check: patch-systems, Type: source > N: > > Have a super great day, > Paul Thanks a lot! Best Regards, kix. -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected] Archive: http://lists.debian.org/[email protected]
