Bug#851359: diffoscope: Improve support for comparing images

Chris Lamb lamby at debian.org
Fri Apr 21 19:58:12 UTC 2017


Hey Maria,

> […]

This is _awesome_! For those who haven't tried this yet, take a look
at:

  http://i.imgur.com/tLHNl5H.jpg


Some very quick thoughts:

- I don't quite follow what Config().html_output is used for (ie.
  needs some comments, etc)

  Perhaps this doesn't need to be a global but rather part of the
  presenter it feels like a bit of an abstraction-layer violation to
  be there.

- Some cosmetic things re. import ordering and superfluous/misleading
  parens, eg.

    - if content_diff is not None:
    + if (content_diff is not None) and Config().html_output
         ^                        ^
 
- You also seem to have the "Emacs"-style Python identation, which
  lines up stuff like so:

               logger.debug('Generating visual difference for %s and %s',
                            self.path, other.path)

  This is suboptimal because changing "logger.debug" means you have
  to adjust the identation of the essentially unrelated line underneath,
  causing unnecessary diff noise.


Regards,

-- 
      ,''`.
     : :'  :     Chris Lamb
     `. `'`      lamby at debian.org / chris-lamb.co.uk
       `-



More information about the Reproducible-builds mailing list