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

checkrhui: Produce TargetUserSpacePreupgradeTasks + refactoring #699

Closed
wants to merge 1 commit into from

Conversation

pirat89
Copy link
Member

@pirat89 pirat89 commented Jul 30, 2021

The RequiredTargetUserspacePackages model has been deprecated
in favor of the TargetUserSpacePreupgradeTasks model. Keeping
the original functionality + produce the new msg as well.

Additionally the actor is a little bit refactored.

@github-actions
Copy link

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergable.
If you want to re-run tests or request review, you can use following commands as a comment:

  • leapp-ci build to run copr build and e2e tests in OAMG CI
  • review please to notify leapp developers of review request

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra.

@pirat89 pirat89 requested a review from Rezney July 30, 2021 09:31
The RequiredTargetUserspacePackages model has been deprecated
in favor of the TargetUserSpacePreupgradeTasks model. Keeping
the original functionality + produce the new msg as well.

Additionally the actor is a little bit refactored.
@pirat89 pirat89 force-pushed the checkrhui-update branch from a473490 to 07a9c5c Compare July 30, 2021 09:40
MichalHe
MichalHe previously approved these changes Aug 2, 2021
Copy link
Member

@MichalHe MichalHe left a comment

Choose a reason for hiding this comment

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

Looks OK to me.

self.produce(RHUIInfo(provider=provider))
self.produce(RequiredTargetUserspacePackages(packages=[info['el8_pkg']]))
self.produce(RpmTransactionTasks(
to_install=[info['el8_pkg']],
Copy link
Member

Choose a reason for hiding this comment

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

Since this is in common, do we want to keep el8_pkg and el7_pkg? Renaming those to old_pkg and new_pkg would make much more sense in common context.

Copy link
Member Author

Choose a reason for hiding this comment

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

@shaded-enmity Good point. We could define the new/old variables in the begining of the begining and than just use these variables everywhere to work with the map in the share library.

@MichalHe MichalHe dismissed their stale review February 10, 2022 20:31

Valid concerns have been requested by other reviewers, therefore, postponing the approval.

@MichalHe
Copy link
Member

/packit build

@packit-as-a-service
Copy link

No config file for packit (e.g. .packit.yaml) found in oamg/leapp-repository on commit 07a9c5c

For more info, please check out the documentation: https://packit.dev/docs/packit-service or contact us - Packit team

@pirat89
Copy link
Member Author

pirat89 commented Feb 11, 2022

@MichalHe you can ignore this one for now. we know it's need to be rebased but currently this is postponed for the month after the release.

@pirat89 pirat89 marked this pull request as draft August 23, 2022 07:31
@pirat89
Copy link
Member Author

pirat89 commented Jan 5, 2023

@fernflower not sure whether the RHUI actors have been already refactored. this should really happen still if they are not using new up-to-date standards instead of deprecated functionality. let's keep it for the next release, where I would like to propose to drop some deprecated functionalities to clean the code.

@pirat89
Copy link
Member Author

pirat89 commented Mar 13, 2023

Closing in favor of PR #1057

@pirat89 pirat89 closed this Mar 13, 2023
@pirat89 pirat89 deleted the checkrhui-update branch February 19, 2024 12:34
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