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. >
