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

Hide/Remove deprecated methods #1294

Merged

Conversation

kontura
Copy link
Contributor

@kontura kontura commented Mar 5, 2024

There is still public libdnf5::repo::Repo::load() which is marked as deprecated but it
cannot be easily hidden because dnfdaemon uses it as well.

@j-mracek since you originally marked it as deprecated do we want to keep the load() method or do we need to do some bigger changes to hide it?

For: #1091

@kontura kontura force-pushed the remove_deprecated branch 2 times, most recently from 204994d to b300528 Compare March 5, 2024 07:26
kontura added 2 commits March 5, 2024 08:29
There is still public `load()` which is marked as deprecated but it
cannot be easily hidden because dnfdaemon uses it as well.
@kontura kontura force-pushed the remove_deprecated branch from b300528 to 900f84f Compare March 5, 2024 07:29
@jan-kolarik jan-kolarik self-assigned this Mar 18, 2024
@jan-kolarik
Copy link
Member

jan-kolarik commented Mar 18, 2024

LGTM.

Regarding the Repo::load() issue, we can address it in a follow-up PR if it's too complex. Currently, it's used in the Context, main and dnf5daemon. Since update_and_load_enabled_repos is expected to be the only method for loading repositories, the only way to load just the system repository seems to be manually disabling available repositories before executing that method, is that right? I'm not sure how common is this use case, but it does feel somewhat clumsy.

@kontura
Copy link
Contributor Author

kontura commented Mar 18, 2024

Regarding the Repo::load() issue, we can address it in a follow-up PR if it's too complex.

A follow-up PR sounds good.

Currently, it's used in the Context, main and dnf5daemon. Since update_and_load_enabled_repos is expected to be the only method for loading repositories, the only way to load just the system repository seems to be manually disabling available repositories before executing that method, is that right?

Yes, without the libdnf5::repo::Repo::load() I think that is the case.

I'm not sure how common is this use case, but it does feel somewhat clumsy.

Both dnf5 and dnf5daemon do it, I think its pretty common. For example for querying just installed packages.

@jan-kolarik jan-kolarik merged commit 6ffa19c into rpm-software-management:dnf5-5.2.0.0 Mar 18, 2024
13 of 22 checks passed
@jan-kolarik jan-kolarik mentioned this pull request Mar 18, 2024
10 tasks
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.

2 participants