Aptdaemon

Code review comment for lp:~mvo/aptdaemon/support-change-credentials-on-add-repo

  1. support-change-credentials-on-add-repo
  2. Merge into main
Revision history for this message
Martin Pitt (pitti) wrote :

> > The rest looks good to me, except potential UnicodeDecodeErrors in the tests
> for non-ASCII characters.
>
> Could you elaborate? Or is that covered by the suggestion to use a
> non-ascii char inside the test?

Yes, it is. I meant that with adding UTF chars the tests might fail.

In http://bazaar.launchpad.net/~mvo/aptdaemon/support-change-credentials-on-add-repo/revision/855, I think it's better to close the temp file instead of just flushing it. Otherwise the open fd will stay around even after renaming.

Thanks for the fixes!

review: Approve

« Back to merge proposal