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

factory-package-news: support newer python3 rpm bindings #3002

Merged
merged 1 commit into from
Aug 4, 2023

Conversation

g7
Copy link
Member

@g7 g7 commented Aug 2, 2023

rpm's python bindings changed in version 4.15 [0] so that they actually return utf-8 strings. Handle this case while keeping support for the older bindings such as the ones shipped with Leap 15.

[0] rpm-software-management/rpm@84920f8

rpm's python bindings changed in version 4.15 [0] so that they actually return
utf-8 strings. Handle this case while keeping support for the older bindings such
as the ones shipped with Leap 15.

[0] rpm-software-management/rpm@84920f8

Signed-off-by: Eugenio Paolantonio <[email protected]>
@g7
Copy link
Member Author

g7 commented Aug 2, 2023

v2: addressed flake8 warnings :)

@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (82c067d) 28.45% compared to head (17f0cd5) 28.45%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3002   +/-   ##
=======================================
  Coverage   28.45%   28.45%           
=======================================
  Files          85       85           
  Lines       14716    14716           
=======================================
  Hits         4188     4188           
  Misses      10528    10528           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

# keep this script working there too.
#
# [0] https://github.com/rpm-software-management/rpm/commit/84920f898315d09a57a3f1067433eaeb7de5e830
def forcestr(content):
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if utf8str would be a slightly besser name.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, yeah definitely looks better than forcestr :) Should I make another PR that changes that?

Copy link
Member

Choose a reason for hiding this comment

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

Yes please if you agree

Copy link
Member

@dirkmueller dirkmueller left a comment

Choose a reason for hiding this comment

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

LGTM, small nit inline

@dirkmueller
Copy link
Member

test are passing, we can solve the nit anytime..

@dirkmueller dirkmueller merged commit 9200c9e into openSUSE:master Aug 4, 2023
8 checks passed
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.

3 participants