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

Enable module to not use default options #1192

Merged

Conversation

morremeyer
Copy link
Contributor

This PR enables you to either use:

  • override_options as it was the case previously. Those options are merged with the default options
  • options. Those options won’t be merged with the default options.

Both are mutually exclusive in usage, meaning that one of them has to be empty.

I wrote this PR because we want to get rid of the version specific configuration options as it makes our workflows for changes of default parameters in our profiles easier.

I wrote it like this so that it has clean semantics:

  • override_options tells you that defaults are overridden
  • options tells you that options will be set

Therefore, this PR is fully backwards compatible.

@morremeyer morremeyer force-pushed the feature/disable_default_conf branch 3 times, most recently from f9de949 to 13ed0e9 Compare May 8, 2019 13:09
@morremeyer morremeyer force-pushed the feature/disable_default_conf branch from 13ed0e9 to 7ac8328 Compare May 27, 2019 11:46
manifests/server.pp Outdated Show resolved Hide resolved
@sheenaajay
Copy link
Contributor

Thanks @mauricemeyer for submitting the PR. Could you please incorporate comments. Thank you @bastelfreak for the valuable comments.

@morremeyer morremeyer force-pushed the feature/disable_default_conf branch from 7ac8328 to 8c88a27 Compare September 18, 2019 12:38
@morremeyer morremeyer requested a review from a team as a code owner September 18, 2019 12:38
@morremeyer
Copy link
Contributor Author

@sheenaajay Thanks for the reminder! I updated the PR, let’s see what @bastelfreak thinks :)

@sheenaajay
Copy link
Contributor

Thank you @mauricemeyer .

@morremeyer morremeyer force-pushed the feature/disable_default_conf branch 2 times, most recently from 4d3f420 to d236908 Compare October 16, 2019 11:11
@morremeyer
Copy link
Contributor Author

The test for Puppet 5 failed, but it looks to me like a test problem?

I can’t restart the test, though.

@bastelfreak
Copy link
Collaborator

@mauricemeyer magic travis trick: close + reopen the PR will retrigger travis.

@morremeyer morremeyer closed this Oct 16, 2019
@morremeyer morremeyer reopened this Oct 16, 2019
@morremeyer
Copy link
Contributor Author

@bastelfreak Works like a charm, thanks.

@lionce
Copy link
Contributor

lionce commented Nov 7, 2019

close and reopen to rerun tests

@lionce lionce closed this Nov 7, 2019
@lionce lionce reopened this Nov 7, 2019
@lionce
Copy link
Contributor

lionce commented Nov 7, 2019

Hello @mauricemeyer ,

It looks good to me! Thank you!
@bastelfreak thank you for your great review! :) Please re-check this

Cheers!

Screenshot 2019-11-07 at 14 06 09

@kubicgruenfeld kubicgruenfeld force-pushed the feature/disable_default_conf branch from d236908 to 1864190 Compare December 13, 2019 14:35
@carabasdaniel carabasdaniel merged commit 775bed0 into puppetlabs:master Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants