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

Hi Sebastien, thank you for looking into it.

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

Let's consider remove_key() defined at softwareproperties/SoftwareProperties.py:838, for instance. The call to self.KeysModified is wrapped in a try-except that catches the AttributeError exception (self doesn't have any KeysModified), and make remove_key always return False.
The same holds for add_key() and update_keys().

> * shouldn't those calls be moved to the Dbus backend

Yes, definitely (imho). But that's a different story. Please, see https://bugs.launchpad.net/ubuntu/+source/software-properties/+bug/1804887 The patch there is just a proof of concept. Since then I ported the whole application to DBus, but did't publish the code yet.

> * 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?

I guess at some point the UI was showing 8 digits keys, therefore :8 was used. Now it shows 16. It is now advised to use the full key fingerprint (40 digits) in order to avoid collision attacks, but that's really long (we should change the UI).
Using :16 was just the smallest change to make it work, with the added benefit of using 64 bits key id.
A way to check fingerprints like in Dolphin's file properties for md5/sha1/sha256 would be cool, where you paste some text and it tells you if it does match.