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

repoquery: add --srpm and --sourcerpm #595

Merged
merged 4 commits into from
Jun 22, 2023

Conversation

kontura
Copy link
Contributor

@kontura kontura commented Jun 7, 2023

For: #122

@kontura
Copy link
Contributor Author

kontura commented Jun 12, 2023

Rebased.


if (querytags_option->get_value()) {
libdnf::cli::output::print_available_pkg_attrs(stdout);
} else if (changelogs->get_value()) {
libdnf::rpm::PackageQuery wrapped(result_pset);
libdnf::cli::output::print_changelogs(wrapped, full_package_query, false, 0, 0);
libdnf::cli::output::print_changelogs(full_package_query, full_package_query, false, 0, 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was confused about this, because the second argument for print_changelogs changed during the move, but it doesn't have any effect... However, the query is then unnecessarily copied and filtered, so wouldn't it be better to pass an empty query or change it in the print_changelogs? Neither is necessary in this PR, though (it's actually smaller now than it was before).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree. I tried to improve the print_changelogs in a new commit. Please take a look.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. FYI, I encountered an unrelated issue with the changelog command (#627) when testing it. Just mentioning if you run into the same.

*this,
"srpm",
'\0',
"For selected packages format corresponding source RPMs (enables source repositories).",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I don't understand the word "format" in this context. Previously, there was "operate on", which seems more intuitive, but maybe I am missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to express that if the --srpm switch is applied we can only format (specify which information to show) the selected srpms but they won't be filtered anymore because all filtering is done before.

To me "operate on" suggest that we can apply also other options to the srpms. Although maybe I am overthinking this 🙂.

What about:

"After filtering is finished use packages' corresponding source RPMs for output (enables source repositories).",

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, other options are applied to the srpms - the query options. In dnf5, I don't see it in the documentation (at least not on the first glance), but there is info or the pkg_attrs_options (requires, provides...).

But I see what you mean and the "operate on" is not super clear either. The proposed sentence looks easier to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated it.

The filters in `print_changelogs(...)` are exclusive, only one can be
applied per call.

This also means we don't have to pass in `PackageQuery` if it is not
needed.
@pkratoch pkratoch added this pull request to the merge queue Jun 22, 2023
Merged via the queue into rpm-software-management:main with commit d2b7152 Jun 22, 2023
@pkratoch pkratoch mentioned this pull request Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants