On Sat, Aug 5, 2017 at 5:12 AM, Mattia Rizzolo <mat...@debian.org> wrote:

On Wed, Aug 02, 2017 at 06:51:00AM -0700, Olzhas Rakhimov wrote:
> > I explicitly listed 'to-be-installed' files in scram.install &
> > scram-gui.install. Reuploaded
>
> Very well, so let's get to the rest of the packaging review:  btw, be
> aware that I am looking only at what's in the git repository, I haven't
> even looked at mentors.d.n.
>
> * I see the bash completion moved from
>   /usr/share/bash-completion/completions/scram.sh to
>   /usr/share/bash-completion/completions/scram/scram.sh - I have no
>   knowledge of how bash completions should be handled, does it still
>   work?  (it's due a trailing '/scram' in d/scram.install, remember that
>   dh_install does not behave like cp…)
>
​
This script with shell extension was somehow failing on Ubuntu​.
I wanted to rename and remove the extension.

* is d/p/0001-GUI-Fix-the-static-build.patch still needed after
>   switching to the dynamic lib again?
>
​You are right it is not needed anymore. I removed it.

* is d/p/0001-GUI-Update-.desktop-with-URL-and-Keywords.patch upstreamed
>   or something?
>
​Yes, this is directly from upstream.
​

* The last debian upload was 0.11.5-1, but there are a bunch of other
>   changelog entries: it's not a real problem (I just need to pass the
>   correct -v option to dpkg-buildpackage (dpkg-genchanges actually) when
>   building before upload), but still weird.  Consider that you could
>   have uploaded all of those to experimental instead of keeping them for
>   now :)
> * stadards-version bump is not documented
> * new binary is not documented either
> * same for d/copyright changes
>
​
Probably, I have to learn how to do uploads to experimental.
For now, I will squash the changelogs for the pseudo-published releases.
​

* all of this said, why was the manpage removed from the upstream side?
>
​The upstream switched to help2man to generate manpages from help prompts
automatically.​

* last: why -DBUILD_TESTS=OFF ?
>
​This depends on custom (patched) googletest submodule that I cannot seem
to pull into ​builds.
I need to download the tarball or with git submodule update --init
--recursive with the original repository.
I hope to drop that custom dependency someday and switch to standard
googletest available on debian;
at that time, it should be easier to port the tests as well.


> As you can see nothing major, in my opinion: nice work :)
>
> ​Thanks for the review. There’s definitely a lot nitty-gritty stuff to
learn for me.​

Regards,
Olzhas Rakhimov
​

Reply via email to