Bug#832941: RFS: 4pane (debian: message 7 of 20)
Dear Sean,
Many thanks for taking the time to write such a detailed review. Of the
must-fixes:
>1. The changelog should contain exactly one entry, closing the ITP.
Ah, that not only makes sense but is easier. Done.
>2. Why doesn't 4pane work on archs other than i386, amd64, hurd-i386?
I didn't think it would. However after discussion on debian-mentors I've now
shown it works on armel, and it builds on kfreebsd-amd64 but immediately hangs
in a forkpty call; I'll try to debug that when I have time. I expect some or
all of the other archs will also build. If/when 4pane attracts a sponsor I'll
try them.
>3 The Vcs-* fields should point to a repo/branch containing the source
>package, not the upstream master branch.
That also makes sense. Done.
>4 I think that it would be clearer to express the wxWindows license as a
>dual-license: "GPL-2+ or wxWindows", with separate license paragraphs for each
>of those.
I'm not sure I understand. wxWidgets isn't dual-licensed, it uses just the
wxWindows license which, though similar to GPL-2, is different.
Of your suggestions, 2, 3, 5, and 7-9 are now implemented. Of the others that
need a comment:
>1 I'd be very grateful if you'd put this source package in a git repository.
I presume you mean just the contents of debian/. I've done this:
https://github.com/dghart/4pane-debian-dir
>4.Why is there an additional copy of the manpage in the debian/ subdir?
That was a quick-fix for a 4[Pp]ane issue. I've now removed it and instead use
a link.
>6. Can Vcs-Git: use a secure protocol? https:// rather than git://.
I don't think so, not without the user logging in to sourceforge.
>10. You're inconsistent about 4pane versus 4Pane...
The original name was 4Pane, and outside debian it still is. I don't mind
changing things while packaging but it risks breakages, so I'll ask my future
sponsor how best to do this.
>11. I'm not familiar with wxWidgets practices, but is it unavoidable to
>include sections of code from the wxWidgets sources...
It's not at all normal, but I have to do it in two places:
First, originally the main display widget was effectively unsubclassable and
even now it would be very hard. I hope to replace the control with one new in
wx3.0 when I no longer support wx2.8. Second, wxWidgets' inotify wrapper is new
in wx3.0; I've copied some of the code for use in earlier versions.
>12. Please consider using dh_autoreconf to ensure that the package's build
>system can be reproduced from the source code provided
If I understand dh_autoreconf correctly, it can't: a fresh autoconf call will
fail as the package doesn't include various non-standard .m4 macros and such.
That could be fixed by a largish patch, but I'll leave it to a sponsor to
decide.
Again, thank you for your time and effort.
Regards,
David Hart
>Dear David,
>
>I've split it into two sections: things that I would consider must-fixes
>before an upload to Debian, and suggested improvements. The latter
>aren't strictly necessary, but they would help demonstrate to a
>potential sponsor that you are committed to maintaining this package in
>Debian.
>
>Must-fixes/clarifications:
>
>1. The changelog should contain exactly one entry, closing the ITP. The
>current changelog suggests that 4pane has already been uploaded to
>Debian.
>
>2. Why doesn't 4pane work on archs other than i386, amd64, hurd-i386?
>(an eclectic list!)
>
>3. The Vcs-* fields should point to a repo/branch containing the source
>package, not the upstream master branch.
>
>4. I think that it would be clearer to express the wxWindows license as
>a dual-license: "GPL-2+ or wxWindows", with separate license paragraphs
>for each of those.
>
>Suggestions:
>
>1. I'd be very grateful if you'd put this source package in a git
>repository. That way, I can use `git diff` to review changes that
>you've made in response to my comments.
>
>2. At least PACKAGERS, README are missing a trailing newline.
>
>3. The "but may be used by others" line in the manpage doesn't make much
>sense. Normally this is used to indicate that a manpage written by a
>Debian contributor is used in a Debian derivative, but of course any
>copy of a manpage written by *upstream* gets used by others.
>
>4. Why is there an additional copy of the manpage in the debian/ subdir?
>
>5. "As well as standard file manager things" I would suggest
>s/things/functionality/. Also, the scare quotes around 'terminal
>emulator', 'grep' etc. aren't needed and could be confusing.
>
>6. Can Vcs-Git: use a secure protocol? https:// rather than git://.
>
>7. The debian/* paragraph in d/copyright is redundant.
>
>8. The Debian menu system is deprecated. You seem to be installing a
>FreeDesktop.org desktop file into /usr/share/4Pane/rc. Please install
>that as a desktop file that will be picked up by desktop environments --
>see Debian Policy 9.6.
>
>9. I think the html docs should go in /usr/share/doc/4pane/html. Then
>someone just looking for the changelog, README etc. need not hunt
>through the html files.
>
>10. You're inconsistent about 4pane versus 4Pane. For example, you use
>/usr/share/4Pane but /usr/share/doc/4pane (with 4Pane as a symlink).
>Since the package is 4pane, you should use '4pane' in all directories
>the package uses (the compatibility symlinks from 4Pane to 4pane are
>fine).
>
>11. I'm not familiar with wxWidgets practices, but is it unavoidable to
>include sections of code from the wxWidgets sources, as you describe in
>LICENSE? Is it not possible to call library functions instead? Since
>you link against libwxgtk I assume that it isn't possible to replace the
>copied code with library calls, but I'd appreciate it if you could
>confirm that.
>
>12. Please consider using dh_autoreconf to ensure that the package's
>build system can be reproduced from the source code provided
>
>I haven't tried installing and running the package yet, but hopefully
>the above is enough to be going on with.
>
Reply to: