Dear Niels,
> I have reviewed your package and there some issues with it. It may
> appear as if I am tearing your package completely apart, but it is
> intended as constructive criticism to improve your package.
Thank you very much for the detailed review!
> The package does not close an ITP (Intend To Package) bug, the new
> maintainer's guide explain how this can be done[1]. This serves a
> number of purposes, such as avoiding duplicate work.
I tried to file a bug today, but it reportbug told me that it was unable
to connect to Debian BTS. I tried with 5.1.1 and 4.12.6ubuntu1.
Then I tried to send a bug report via email:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=636295
> I wonder if the synopsis could be extended a bit to give a clear idea of
> what the application does. Personally, "Infinite Notepad" made me think
> it was a "Windows Notepad" clone or something similar.
You are right, I changed it to "graphical notepad for use with a pen
tablet".
> The source package has a warning about the Standards-Versions being out
> of date. I can recommend using the upgrade check list (in the
> debian-policy package or at [2]). Ignore the "Unreleased" note under
> 3.9.2, the maintainers forgot to remove it in the last upload.
Are the package supposed to be build on unstable? I currently build it
on Ubuntu 11.04 for which the version is the current standard.
> If you did not know it, lintian can be run on .dsc-, .(u)deb- and
> .changes-files. When run on .changes-files it will process all files
> listed in it. Generally lintian gives the best results if all the
> packages are processed together.
The only lint it finds is that there is no upstream changelog.
> What about future motivation?
> - Would you be willing to support jscribble in Debian for 2 years?
> (This is the average length of a stable release)
Since I plan on using this software for the next years, I would be able
to support it.
> - Do you see yourself working on other parts of or packages in Debian?
> (i.e.: would you be interested in joining the Debian Java Team?)
I will have to see how much time I have once university starts.
> On some platforms, default-jdk has the version "2:1.5", which would give
> you a 1.5 Java. You may want to consider using
> openjdk-6-{jdk,jre} | sun-java6-{jdk,jre} instead.
Somebody told me that using default-jre was preferred since it does not
tell people which jre to use. So I would use openjdk in this case to
prevent this quirk of messing with the package?
> License: None of the files in the upstream part (that is anything not in
> the debian directory) appears to have any licenses, nor does the package
> carry a license. Since you are (also) the upstream maintainer, this
> should be trivial to fix.
I included the header in all the source files and put a full text into
the directory.
> d/watch: Missing - while you are your own upstream, it is still a good
> custom to have a watch file. Particularly, if you later step down from
> either position, we will have tools to detect if the package is out of date.
This I did not fix yet, I will look into it in the future.
> tests: There are tests in the package, but they do appear to be run.
> Can they be run during the build? debhelper should be able to pick it
> up if the makefile has a "test" or a "check" target. (Remember to
> update Build-Depends accordingly).
I created a target that exercises all unit tests to the makefile. Junit
is now also part of the dependencies.
> native: The package has been created as a native package, but this
> package is not a "By Debian, For Debian" package. See [4] for more
> information.
Okay, I changed it to non-native.
> d/install: You should probably use d/manpages for the manpage (see
> man dh_installmanpages).
This is way better than my workaround with the makefile. Thanks!
> Also, the jar package is installed into
> /usr/share/java, which is generally reserved for public java libraries.
> /usr/share/jscribble/ is probably what you want (remember to update
> install_files/jscribble accordingly).
I changed it, thanks for pointing to the launcher script as well!
> d/docs: Why do you install the manifest? I do not see how any regular
> user would find it useful.
Honestly, I have no idea how it got into there. It is now removed.
> d/control [pedantic]: On a more pedantic note, why the Build-Depends on
> debhelper (>= 7.0.50~). 7.0.50~ is only needed if use override targets
> (which you do not), so a >= 7 would work just fine. Alternatively
> debhelper 8 is out, so you can (if you like) bump the compat level (and
> the Build-Depends) to 8.
Ubuntu even ships 8.1.2. Is there any advantage of doing so? I guess it
would be easier to backport if the version is as low as possible.
> I hope you find the feedback useful. :)
Very much so!
I uploaded version 1.0.1-1 of the package now.
Regards,
Martin
--
The package can be found on mentors.debian.net:
- URL: http://mentors.debian.net/debian/pool/main/j/jscribble
- Source repository: deb-src http://mentors.debian.net/debian unstable
main contrib non-free
- dget
http://mentors.debian.net/debian/pool/main/j/jscribble/jscribble_1.0.1-1.dsc
Attachment:
signature.asc
Description: OpenPGP digital signature