Code review comment for ~chad.smith/ubuntu/+source/update-notifier:xenial-esm-product-url-renaming

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

This almost LGTM now
What we have discussed now is good, but you added further changes without mentioning. (The changes are ok, but you need to mention them so that everyone is aware and later people can trace it back)
- two changelog updates are left
  a) you have
    - INSTALL, data/hooks.py, tests/test_package-data-downloader.py:
      Fix E117 over-indented pep issues. [ Gianfranco Costamagna ]
    The comment above matches ac0efb8ee21, but then 256bd2635e appears left being unmentioned
    So IMHO you'd need to add another line like:
    - data/apt_check.py: fix over-indent pep8 errors
  b) commit c1a57c8e09 is ok, but you'll need in changelog
    - d/control: add explicit pep8 build-requires dependency
- Optional: squash the two changelog commits that you have

review: Approve

« Back to merge proposal