Re: RFS: wmaker
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.
Reply to: