On Sun, 2016-06-19 at 00:32 +0530, Satyam Zode wrote: > I made the changes as you mentioned, please find an attached patches.
Personally, I would suggest that the changes you have separated into separate patches per file are a single logical change and as such should all be made in the same commit. Some discussion of this "logical change" concept from the OpenStack community is here: https://wiki.openstack.org/wiki/GitCommitMessages > I have removed this temporarily! we already had discussion regarding > this on IRC. Looking forward to your response! I would suggest leaving the range completers in, even with the enforced sorting by bash itself, I think they are useful. For the upper end of the completion range, I think go with double the current maximum. Some further comments on the patches below: > +dh_auto_build: > + debian/diffoscope.bash-completion > + debian/diffoscope.1 This will cause the build to fail, those three lines need to all be on one single line with spaces in between. Did gmail wrap it for you? dh_auto_build: debian/diffoscope.bash-completion debian/diffoscope.1 > +++ b/debian/clean > @@ -0,0 +1,3 @@ > +rm -f debian/diffoscope.1 > +dh_clean -O--buildsystem=pybuild The first two lines of the debian/clean file need fixes. See the manual page for dh_clean, but the debian/clean file is simply a list of files (one per line) to be removed by dh_clean from `debian/rules clean`. The full debian/clean file for diffoscope should look like this: debian/diffoscope.1 debian/diffoscope.bash-completion > + elif '_ARGCOMPLETE' not in os.environ: This will trigger the error in the circumstance where you don't have argcomplete installed and you aren't asking for completion. This means it will give an error when running it from the command-line if argcomplete isn't installed. I think we want an error when you don't have argcomplete installed but you are asking for completion. To fix this, remove the "not" from this line. > + print('ERROR: Argument completion requested but Python argcomplete > module not installed', file=sys.stderr) In another place in the diffoscope codebase, a critical error is reported using logger.critical before exiting with an error. I'm not sure if print, logger.critical or logger.error should be used for this. I guess Lunar or other folks can advise you about this. logger.critical('Console is unable to print Unicode characters. Set e.g. LC_CTYPE=en_US.UTF-8') -- bye, pabs https://wiki.debian.org/PaulWise
signature.asc
Description: This is a digitally signed message part
