Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify pubkey header building code #1625

Merged
merged 1 commit into from Apr 9, 2021
Merged

Simplify pubkey header building code #1625

merged 1 commit into from Apr 9, 2021

Conversation

dmantipov
Copy link
Contributor

At makePubkeyHeader(), the key has PGP data collected already,
so rpmPubkeyDig() is redundant. And, since the former is the only
user of the latter, which, in turn, mostly duplicates the
functionaliry of rpmPubkeyNew(), rpmPubkeyDig() may be dropped.

@pmatilai
Copy link
Member

pmatilai commented Apr 8, 2021

rpmPubkeyDig() is a public API function, we can't just drop it. At least current incarnations of libdnf actually even use it. Quite possibly for various wrong reasons, but still.

@DemiMarie
Copy link
Contributor

We can’t drop it, but we can certainly stop using it internally :)

At makePubkeyHeader(), the key has PGP data collected already,
so rpmPubkeyDig() is redundant.
@dmantipov
Copy link
Contributor Author

We can’t drop it, but we can certainly stop using it internally :)

OK let's reduce the change to internal scope.

@dmantipov
Copy link
Contributor Author

rpmPubkeyDig() is a public API function, we can't just drop it.

How many (important) API dependencies we have? What if I do the corresponding patch for libdnf as well?

@pmatilai
Copy link
Member

pmatilai commented Apr 9, 2021

It's not a question of any individual user. Removing interfaces from public API requires a soname bump and those are not something to be done lightly because they cause pain to every single rpm (API) user. So it's not something you'd do for any one interface. What we can do is mark stuff deprecated and then remove those in a batch when there's enough of a reason for the soname bump.

@pmatilai
Copy link
Member

pmatilai commented Apr 9, 2021

Anyway, thanks for the patch!

@pmatilai pmatilai merged commit 4afe2d1 into rpm-software-management:master Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants