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

Bug#888807: RFS: qstardict/1.3-1



X-Debbugs-CC: tobi@debian.org
Control: tag -1 - moreinfo

Hi tobi,

Thanks for your review! In fact I didn't receive your reply before
(don't know why) and I just noticed it via BTS web interface. Anyway
here's the updated status:

> - small typo in d/copyright: Alexander had maintained the package in
>   2007 and 2008. Also it should be "Comment:" (singular)

Done.

> - Please review d/copyright. I found at least one file which is not
>   properly recorded (wrong license and wrong copyright holder)

Done. I looked into every source files in the repository this time.

> - don't install README.md -- it does not have extra information beyond
>   a package  description and compilation instructions (which are useless
>   for the users of the binary package)
>   There is also a slight bug in it: The URLs at the bottom seems
>   outdated, they will forward to the github project from the watch file.
>   Maybe at least report that to upstream.)

Done. The typo was forwarded upstream and got fixed in trunk code.

> - Please upstream the manpage (Alexander as upstream should include it
>   there so that other distributions will also benefit from it)

I've filed an issue on upstream GitHub project.
https://github.com/a-rodin/qstardict/issues/19

> - The embedded libqxt -- can you use the Debian packaged version?

Sorry but nope -- If we take a look into libqxt in Debian, #875027 says that
libqxt is unmaintained upstream and will be removed from Debian archive
soon. Upstream git repository also suggested that all projects previously
using libqxt should either migrate away from libqxt or embed part of its
code to fit their own need. [1] That is exactly what qstardict
upstream is doing,
see also the GitHub issue [2].

[1] https://bitbucket.org/libqxt/libqxt/wiki/Home
[2] https://github.com/a-rodin/qstardict/issues/16

> - Some lintian stuff:
> N: Processing binary package qstardict (version 1.3-1, arch amd64) ...
> I: qstardict: spelling-error-in-binary usr/bin/qstardict writen written
> I: qstardict: spelling-error-in-binary
> usr/lib/qstardict/plugins/libstardict.so wil will
> I: qstardict: spelling-error-in-binary
> usr/lib/qstardict/plugins/libstardict.so formated formatted
> I: qstardict: desktop-entry-lacks-keywords-entry
> usr/share/applications/qstardict.desktop
> (spelling errors should be at least sent upstream, but they
> are quite easy to fix and then a patch can be sent upstream :))
> note that the spelling errors might also needs fixing in the
>         translation templates.

Fixes are submitted upstream and got merged. Patches are also cherry-picked
in debian/patches directory.

> - check-all-the-things also found a bit of stuff.

I took a look and forwarded the information from cppcheck and flawfinder
to upstream.

https://github.com/a-rodin/qstardict/issues/17

- The watch file is not working.

Fixed.

> Future homework (optional -- bonus points area ;-))

I decided not to do them this time -- will come back to them after I get to
know qstardict better with a period of app using experience.

The new version is now uploaded onto mentors.debian.net and
salsa.debian.org/debian/qstardict repository.

--
Thanks,
Boyuan Yang


Reply to: