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 common utility code to plugins.module_utils.util #390

Merged
merged 3 commits into from
Jun 20, 2022

Conversation

felixfontein
Copy link
Collaborator

SUMMARY

Subset of #387.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

various modules and plugins

Copy link
Contributor

@briantist briantist left a comment

Choose a reason for hiding this comment

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

You may want to consider making the util internal so that "breaking" changes in the util interface that don't break things in the user-facing code (because it's changed at the same time), will not need to be a breaking change in the collection.

Details in this comment (it's your minimized comment, need to expand to see it):

(I did this for all of mine, since I don't expect or want any other collections to be depending on them)

@briantist
Copy link
Contributor

Ah, I've just realized that that stuff was mostly (completely?) moved from a different module_util, not from the modules themselves. I guess technically this is a breaking change then, though I'm personally not interested in being a stickler about that when we have a pretty good idea of when a module util is used internally only... I don't have such an idea on this one but I trust your judgement on it.

@felixfontein
Copy link
Collaborator Author

It's not a breaking change because it's still available in the other module_utils (due import).

If you move a function (or class) x from module_utils a to module_utils b and add from b import x into a, x is now available from both.

Copy link
Contributor

@briantist briantist left a comment

Choose a reason for hiding this comment

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

I see, I missed that detail

@felixfontein felixfontein merged commit f41d7ac into ansible-collections:main Jun 20, 2022
@felixfontein felixfontein deleted the util branch June 20, 2022 16:39
@felixfontein
Copy link
Collaborator Author

@briantist thanks a lot for reviewing this :)

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.

2 participants