On Sun, 2016-06-12 at 11:50 +0530, Satyam Zode wrote:

> Tags: patch

That only works in mails to sub...@bugs.debian.org, for mails to the
bug report address, you want this instead (-1 means the current bug):

Control: tags -1 + patch

> I have updated the patch as per suggestions please review it.

Review below...

>  override_dh_clean:
> +     debian/diffoscope.bash-completion

This causes the package to fail to build. I think override_dh_clean
should be removed and both the bash completion script and
debian/diffoscope.1 added to debian/clean. Moving the existing things
to the debian/clean file should be done in a separate commit.

>       dh_python3 --recommends=python-debian --recommends=rpm-python 
> --recommends=tlsh --recommends=guestfs

This line needs to add --recommends=argcomplete

> +override_dh_auto_build:
> +     register-python-argcomplete diffoscope > 
> debian/diffoscope.bash-completion

This disables the upstream build system, you should call dh_auto_build
after calling register-python-argcomplete, I think this is a better way
to do it than what is there now:

override_dh_auto_build: debian/diffoscope.bash-completion debian/diffoscope.1

        register-python-argcomplete diffoscope > $@

You also need to add generated files to .gitignore so they don't get
accidentally committed to the repository.

> +* ``python-argcomplete`` is used for argument completion.
> +  Available on Debian and Fedora as
> +  ``python-argcomplete``.
> +  ``python-argcomplete`` is also available on `PyPI ` as
> +  ``argcomplete``.

I think I would write that like this:

* ``argcomplete`` is used for argument completion.
  Available on Debian as ``python3-argcomplete``.
  Available on Fedora as ``python-argcomplete``.
  Available on `PyPI <https://pypi.python.org/pypi/argcomplete/>`_.

> -                        default=Config.general.max_report_size)
> +                        
> default=Config.general.max_report_size).completer=RangeCompleter(0,
> +                        Config.general.max_report_size, 200000)

I think this prevents people from completing values above the
default max_report_size? Same for the other RangeCompleters.

Interestingly, in the range completers, 1000000 appears to sort before
800000, so it is doing string sorting not numeric sorting.



Attachment: signature.asc
Description: This is a digitally signed message part

Reproducible-builds mailing list

Reply via email to