Hi Paul,
I've finally had time to sit down to try and fix all the issues with
this package. Hopefully I've had some success, but it would be great if
you could cast your eye over my fixes :)
The package is available here:
http://mentors.debian.net/package/rubyripper
On 04/03/13 00:33, Paul Wise wrote:
> Please get the patches include upstream and or change them to be
> acceptable upstream.
I have 16 patches against the upstream stable tree (though half are
translation lint), and will address each of these:
1. Debian build-system specific.
2. Submitted, not yet accepted upstream.
3. Submitted, not yet accepted upstream.
4. Fixed in upstream master (but not in stable branch).
5. Fixed in upstream master (but not in stable branch).
6. Fixed in upstream master (but not in stable branch).
7 - 15. Translation fixes submitted, not yet accepted upstream.
16. Submitted, not yet accepted upstream.
>
> Please use this watch file instead:
>
> version=3
>
opts=uversionmangle=s/(\d)[_\.\-\+]?((RC|rc|pre|dev|beta|alpha|b|a)\d*)$/$1~$2/
> \
> http://code.google.com/p/rubyripper/downloads/list?can=1 \
> .*/rubyripper-(\d\..+)\.(?:zip|tgz|tbz|txz|(?:tar\.(?:gz|bz2|xz)))
This didn't work for me, but I used the Google Code watch example from
the wiki[0]. Actually, the example has a bug where it looks for two
adjacent periods in the file name. I note that you were the last editor
of the page - maybe you could fix the example? (I don't have edit access)
... .*/<project>-(\d\S*)\.\.(?:zip|tgz|tbz|txz|(?:tar\.(?:gz|bz2|xz)))
^
only one period required here
>
> Please call dh_auto_configure in override_dh_auto_configure instead of
> ./configure, this will allow you to drop --configure.
Fixed.
>
> There doesn't seem to be a need for the rubyripper.png symlink, what
> is that needed for?
Fixed. I'm not sure why I did that in the first place.
>
> The comments in the manual page aren't needed.
Fixed.
>
> Please get the manual page included upstream.
Submitted, not yet accepted upstream.
>
> Please run wrap-and-sort -sa so diffs of debian/* are more human-readable.
Fixed. Thanks for introducing me to this tool!
>
> The package descriptions are almost duplicates of each other.
Is this an issue? I modelled the descriptions on other packages which
have both a command-line and graphical interface, such as vim/vim-gtk.
>
> The SVG source for rubyripper.png is missing from the package.
This is not included upstream either. Does Debian have a policy of not
accepting rasterised icons?
>
> The upstream README contains install and MacOS information, please ask
> upstream to split that out into separate files since it isn't useful
> for Debian users.
When I get the chance, I'll split this information out and submit a patch.
>
> Several of the files in the source are executable, but do not need to be.
Fixed.
>
> I'm not familiar with ruby, but the way it uses popen doesn't look
safe to me.
No, it doesn't look safe to me either (though I hadn't noticed
previously). See patch 6.
It's been fixed differently upstream, but as part of extensive
refactoring which would be too difficult to try to pull into a patch
against the stable version.
>
> Automatic checks:
>
> https://wiki.debian.org/HowToPackageForDebian#Check_points_for_any_package
Thanks for this link, I wasn't aware of it before.
>
> ruby configure --update-lang:
>
> lots of obsolete msgid warnings
See patch 2.
>
> lintian:
>
> P: rubyripper-gtk2: no-upstream-changelog
> P: rubyripper-cli: no-upstream-changelog
There is no changelog included upstream.
>
> cme check dpkg-control:
>
> Says that the short descriptions for the packages are too long.
Fixed.
>
> msgfmt:
>
> many empty msgstr warnings
I'll file bugs upstream for these; I'm sure the results would be tragic
if I tried to fix these using google translate :P
>
> POFileChecker:
>
> lots of warnings:
> missing :
> missing .
> missing ...
> extra \n
>
See patches 7 - 15.
Thanks again for the review. Discovering all these techniques for
de-linting packages has been very educational!
Regards,
Scott Leggett.
[0] https://wiki.debian.org/debian/watch/
--
Regards,
Scott Leggett.
Attachment:
signature.asc
Description: OpenPGP digital signature