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

Use a mixin module instead of inheritance #7

Merged
merged 4 commits into from
Feb 28, 2022
Merged

Use a mixin module instead of inheritance #7

merged 4 commits into from
Feb 28, 2022

Conversation

m0dular
Copy link
Contributor

@m0dular m0dular commented Feb 28, 2022

Use a mixin module instead of inheritance

Prior to this commit, the module used a Puppet class to instantiate a
base provider for the other providers to inherit.  However, this caused
issues when using the 'require' metaparameter in manifests, because
Puppet may not have autoloaded the base type yet.  It was also overly
complicated.

This commit moves this functionality to a shared module which uses
constants and instance methods, which is a simpler design.  This removes
the need for the Puppet wrapper class, and the user does not need to
require anything to use the types.

@m0dular m0dular requested a review from a team as a code owner February 28, 2022 02:18
@puppet-community-rangefinder
Copy link

influxdb::from_toml is a function

that may have no external impact to Forge modules.

influxdb::retrieve_token is a function

that may have no external impact to Forge modules.

influxdb::to_toml is a function

that may have no external impact to Forge modules.

This module is declared in 0 of 578 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

Copy link
Contributor

@MartyEwings MartyEwings left a comment

Choose a reason for hiding this comment

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

Looks good to me with the exception of the commented out code parts.

I also updated my original Pull for testing to add in the missing fixture, I'm not sure if you have to replay that commit over the top of this or not

manifests/init.pp Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
lib/shared/influxdb.rb Outdated Show resolved Hide resolved
@m0dular m0dular changed the title Mixin module Use a mixin module instead of inheritance Feb 28, 2022
MartyEwings and others added 4 commits February 28, 2022 13:45
Prior to this commit, the module used a Puppet class to instantiate a
base provider for the other providers to inherit.  However, this caused
issues when using the 'require' metaparameter in manifests, because
Puppet may not have autoloaded the base type yet.  It was also overly
complicated.

This commit moves this functionality to a shared module which uses
constants and instance methods, which is a simpler design.  This removes
the need for the Puppet wrapper class, and the user does not need to
require anything to use the types.
@m0dular
Copy link
Contributor Author

m0dular commented Feb 28, 2022

Cancelled the tests since it was just a squash.

@m0dular m0dular merged commit 3c2ea78 into main Feb 28, 2022
@MartyEwings MartyEwings added the enhancement New feature or request label Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants