Code review comment for lp:~silhusk/software-properties/lp1656100-remove-signing-keys

Revision history for this message
Sebastien Bacher (seb128) wrote :

Thank you for the work there, I'm not really familiar with that code but it has no dedicated maintainer so I'm trying to help with review, some comments/questions

* you wrote 'Remove calls to KeysModified which doesnt's exist and causes all actions on
  keys to look like they failed.'

Where does it look like they failed? What actions are you talking about? (unsure why there is no error/traceback printed about those calls)

* shouldn't those calls be moved to the Dbus backend rather than removed, like it was done in http://launchpadlibrarian.net/91837493/software-properties_0.82.3_0.82.4.diff.gz

* apt-key list is listing 8 digits keys, should we do the same on the UI? (but the parsing code doing [:8] takes the wrong half it seems, did that ever worked? [8:16] looks like what we want or just using :16 should be good enough?

(it doesn't help that "apt-key rm <garbage>" always returns "OK" (known as https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=799342)

review: Needs Information

« Back to merge proposal