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

Move util.go to miq-component/utils subpackage #993

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nasark
Copy link
Member

@nasark nasark commented Sep 25, 2023

Currently if util functions are needed elsewhere, the entire miq-components package needs to be pulled. It would be nice to be able to pull in just the util methods, this is especially useful for the downstream operator

@miq-bot assign @Fryguy
@miq-bot add_reviewer @bdunne
@miq-bot add_labels enhancement, refactoring

@nasark nasark requested a review from bdunne as a code owner September 25, 2023 19:48
@nasark
Copy link
Member Author

nasark commented Sep 25, 2023

Marking as WIP since I'm not exactly sure how to test this before merging

@nasark nasark force-pushed the move_utils_to_subpackage branch from e383502 to 7f9695a Compare September 27, 2023 19:43
@miq-bot
Copy link
Member

miq-bot commented Sep 27, 2023

Checked commit nasark@7f9695a with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. 🍪

@nasark nasark changed the title [WIP] Move util.go to miq-component/utils subpackage Move util.go to miq-component/utils subpackage Sep 27, 2023
@nasark
Copy link
Member Author

nasark commented Sep 27, 2023

Successfully tested this by swapping all github.com/ManageIQ/manageiq-pods references with my fork github.com/nasark/manageiq-pods, then pulling the package downstream using the commit hash go get -d -u github.com/nasark/manageiq-pods/manageiq-operator@<commit-hash>

@miq-bot miq-bot removed the wip label Sep 27, 2023
@Fryguy
Copy link
Member

Fryguy commented Sep 29, 2023

Nice - Looks good to me.

I did expect more "deletes" where copy-pasted methods were switch to using the common methods, or is that a follow up?

@Fryguy Fryguy assigned bdunne and unassigned Fryguy Sep 29, 2023
Copy link
Member

@Fryguy Fryguy left a comment

Choose a reason for hiding this comment

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

@bdunne Please review.

@bdunne bdunne mentioned this pull request Oct 17, 2023
@miq-bot
Copy link
Member

miq-bot commented Oct 31, 2023

This pull request is not mergeable. Please rebase and repush.

@miq-bot
Copy link
Member

miq-bot commented Feb 5, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

3 similar comments
@miq-bot
Copy link
Member

miq-bot commented May 6, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

@miq-bot
Copy link
Member

miq-bot commented Aug 12, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

@miq-bot
Copy link
Member

miq-bot commented Nov 25, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants