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

Remove rpm pkg from version results(dnf issue:1961) #1607

Conversation

chenhaixing123
Copy link

@inknos inknos self-requested a review July 17, 2023 13:12
@inknos
Copy link
Contributor

inknos commented Jul 17, 2023

Hi, thank you for the PR.

I am thinking whether this is a critical change from the perspective of the longevity of the project. DNF is being replaced by DNF5, that means that the maintenance on this project is being decreased.
Furthermore, the change would have to be ported to RHEL and other dnf-based distros. At this point, I don't find an added value for changing this output. (Keep in mind that some people could parse this output and some scripts could break for this change. We don't know how many scripts are breaking until we change a behavior that breaks them)

Sadly, my opinion is to reject this PR for the reasons stated above.
I am up for discussion before closing it, though; if you have an opinion on it, please, let's talk about it.

@chenhaixing123
Copy link
Author

Thank you for your reply.
At first I was puzzled when I saw this issue.Why the RPM version is also displayed when the DNF version is queried?
Before submitting the PR, I analyzed the impact of the modification.
"history_record_packages" is defined in libdnf and is used in two places in dnf.
1、The output of querying the DNF version is affected.
print_versions(self.base.conf.history_record_packages, self.base,
2、"history_record_packages" is assigned to "using_pkgs_pats", but "using_pkgs_pats" may not actually be used by the final beg function.
https://github.com/rpm-software-management/dnf/blob/9e8ec4848a16bd4e4603599908a5e4fa835192e7/dnf/base.py#L1097
Above, this commit may not affect other functions of dnf.

But I share your concerns.

@inknos
Copy link
Contributor

inknos commented Jul 18, 2023

@chenhaixing123 thank you so much for the investigation. I still think it's better to avoid this change because it's not critical. I would rather focus on dnf5's behavior, which can be ultimately improved, and it will be the future of dnf.

I would close this, and I would invite you to contribute to the new project, which, I believe, would bring more benefit to the community and would benefit of future support.

@chenhaixing123
Copy link
Author

I agree with you.

@inknos
Copy link
Contributor

inknos commented Jul 18, 2023

I agree with you.

I'll close it then.

Please don't hesitate to open more PRs and Issues in the future. We welcome your opinions and ideas.
Cheers

@inknos inknos closed this Jul 18, 2023
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

2 participants