[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#832941: RFS: 4pane (debian: to exclusive)



Dear David,

Here's another review, based on your 4pane-debian-dir repo.

Must-fixes
----------

1. At least one of the files added by AddExtraM4Files.patch isn't
accounted for in d/copyright.

2. Vcs-* still point to the upstream code, not your 4pane-debian-dir
repo.

3. d/copyright says that only you hold copyright on MyGenericDirCtrl.cpp
and sdk/fswatcher/*, but the file headers have several other people's
names.  They should all be in d/copyright.

4. Your own copyright is not all 2016.  Some files are 2014, and
About.htm says 2007--2016.

5. Alberto Griggio and Otto Wyss also hold copyright on MyTreeCtrl.*

6. config.guess, config.sub, configure, configure.in, Makefile.in and
install-sh are not accounted for in d/copyright.

7. Some bitmaps aren't accounted for in d/copyright.  For example, I'm
guessing you didn't produce bitmaps/libreoffice.png yourself (unless you did?).

8. Further, could you confirm that, for the files in bitmaps/ and the
images in doc/ whose preferred format for modification is not .png, the
.xpm/.svg/whatever source is available?  I see some .xpm/.svg files but
not for every file.  If the preferred format for modification for those
other files is editing the .png then it's fine.

9. Lots of files in .build/ are not accounted for in d/copyright.

10. .build/DONT_README (heh) explains that the Bakefile tool is required
to regenerate the build system.  I.e. the preferred format for
modification of the buildsystem is not by editing Makefile.in, but by
changing some other files and running Bakefile (is Makefile.in the only
file that Bakefile outputs?).

As before, this is a violation of DFSG.  I think you need to package
Bakefile for Debian, unless you can think of a way around this.

11. You need to run dch -r to refresh the changelog timestamp so it is
after the various changes you've made recently.

Command I used to find the copyright issues: `licensecheck --copyright -r .`

Suggestions
-----------

1. You might raise the debhelper compat to 10, the latest version.  That
means you can drop '--parallel --with autoreconf' which are automatic at
compat 10.

2. At the top of d/rules you define various EXTRA_ env vars.  Could you
explain further why you can't do this "the standard way"?  Would it be
possible to make it work by patching the upstream Makefile?  Generally
speaking, it's better to have a short d/rules and move logic into the
upstream Makefile (otherwise someone trying to understand it has to flip
back and forth between two files).

3. In a previous e-mail I explained why I think it would be clearer to
call the wxWindows license "GPL-2+ or wxWindows-3.1+".  So far as I can
tell you never responded to that -- please consider the points I made.
Unless I'm missing something, it would make d/copyright clearer.

4. You should probably s/4pane/4Pane/ in 4pane.doc-base.

5. Rather than installing 4Pane.desktop twice, you could do something
like this:

    override_dh_install:
        dh_install
        ( cd debian/4pane/usr/share && mv 4Pane/rc/4Pane.desktop applications )

On Sun, Nov 20, 2016 at 06:31:33PM +0000, David Hart wrote:
> Dear Sean,
> 
> >1. Why do you have a folder full of patches, when you are the upstream
> >author of 4Pane?  Have they been applied upstream, but there hasn't been
> >a release of 4Pane recently?
> 
> Yes. They are implementing your previous suggestions. There hasn't been a
> recent 4Pane release and, unless needed for debian packaging reasons, there
> won't be one soon.

A new release is not needed.  Thanks for the explanation.

> >2. I see that w.r.t. 4pane/4Pane, you're using 4Pane wherever Debian
> >policy permits you too.  Good idea.  The only thing I'm not sure about
> >is the symlink from /usr/share/doc/4Pane to /usr/share/doc/4pane/html.
> >It could be misleading, since Debian users expect /usr/share/doc/foo to
> >give them a folder containing a changelog, a Debian changelog etc.  Why
> >not link to /usr/share/doc/4pane?
> 
> IIUC the current situation is correct: the debian changelog etc files are in
> /usr/share/doc/4pane/ as expected, while the symlink from /usr/share/doc/4Pane
> to html/ allows the program to find its F1 help files (which can be accessed
> outside the program using a doc-base reader).
> 
> If this is wrong please tell me.

6. It's definitely not wrong, but it might not be optimal.  What I'm
imagining is the following situation: Debian users expect to find
certain files (changelogs, copyright files) in /usr/share/doc/foo.
Since 4Pane is known as '4Pane', a user might reasonably look in
/usr/share/doc/4Pane.  But then they wouldn't be able to find the
standard files.  For this reason, I think /usr/share/doc/4Pane should be
a link to /usr/share/doc/4pane and 4Pane can be patched to find its help
files there.

-- 
Sean Whitton

Attachment: signature.asc
Description: PGP signature


Reply to: