@pmatilai commented on this pull request.
> @@ -10,40 +10,44 @@ namespace rpm {
class keystore {
public:
+ rpmRC delete_key(rpmtxn txn, rpmPubkey key);
This non-virtual method in an otherwise abstract class makes some bells
hesitantly jingle in my head.
After staring at it for some 15min, I think it comes down to this: the reason
for this refactor here is to get keystore_rpmdb and keystore_fs delete_key() to
share the logic of shorter keyids. But, that logic is intentionally missing
from keystore_openpgp_cert_d because there will never be any short keyids in
that store.
Which is to say, delete_key() should remain virtual, because the logic is not
identical between implementations even to begin with. Only fs and rpmdb
keystores need that short keyid logic, so perhaps the answer really is to use a
friend function to share that logic.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/4005#pullrequestreview-3338857956
You are receiving this because you are subscribed to this thread.
Message ID: <rpm-software-management/rpm/pull/4005/review/[email protected]>_______________________________________________
Rpm-maint mailing list
[email protected]
https://lists.rpm.org/mailman/listinfo/rpm-maint
