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

Don't detect namespaced variables as facts and fixup for puppet-lint >= 2.4 #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alexjfisher
Copy link

Fixes #9
Fixed #10

Puppet-lint 2.4 contained a breaking change in
rodjek/puppet-lint#846

Fixups have already been applied in other plugins such as
mmckinst/puppet-lint-legacy_facts-check#32

This commit reverts ee3a6b0
updates the minimum puppet-lint version to 2.4, and fixes up a couple of
tests where the location of the warnings have changed by one character.
@MartyEwings
Copy link

@mmckinst is this something we could get merged in and a release completed?

@mmckinst
Copy link
Owner

mmckinst commented Feb 9, 2023

I'm trying to get permission from my employer to update it. If I'm unable to get their permission I will donate it to vox pupuli.

@MartyEwings
Copy link

thank you!

@alexjfisher
Copy link
Author

@mmckinst,
Hello there! If you decide to pass on the maintenance of this plugin to Vox Pupuli, please know that you will retain full copyright of all the code you have written, and the license will remain unchanged. We would also like to add you as a collaborator, so that you can continue to review and merge pull requests, if you're interested.

Our biggest issue with the plugin right now is that it requires puppet-lint < 3.0, which is preventing us from making progress. We don't want to give up on the plugin altogether, so our current plan is to include the plugin's check into a new voxpuli-puppet-lint-forks gem.

While we'd prefer not to do this, we won't make any changes until at least the end of this month. Also, this is something we can reverse later if you decide to maintain the gem yourself, or if you decide to migrate it to Vox Pupuli in the future.

Thank you for your time, and please let us know if you have any questions or concerns.

@chelnak
Copy link

chelnak commented Feb 15, 2023

@mmckinst Appreciate your response on this!

How would you feel about us (Puppet) bringing in your plugin in to puppet-lint as a default check?

@mmckinst
Copy link
Owner

@alexjfisher

I'm still going through the process to get approval from my employer. It will most likely come through but will be limited to minor patches for bug fixes or compatibility issues which should cover the maintenance needs of this plugin. It's not that I don't want to update it or maintain it, it is just my hands have been tied legally since 2020 and a new process has just been introduced recently. I should have an answer by the end of the month.

Regardless of that, I don't want to hold back the puppet community from making progress and I feel bad for not being able to update this for so long already.

You have my support to fork this to Vox Pupuli. I will update the README to point to the new home.

I'm not sure of how the fork will impact things already pulling this in their Gemfile. Will they just break once puppet-lint is incremented to 3.0 and they'll hopefully find the README pointing them to the new home and new rubygem name? Is there a way to make the transition smoother?

@chelnak

How would you feel about us (Puppet) bringing in your plugin in to puppet-lint as a default check?

I'm fine with this too.

@ekohl
Copy link

ekohl commented Feb 20, 2023

@mmckinst how would it practically go? I noticed it's all your code, except for 61174be so would it be sufficient for you and @seanmil to agree to relicense the code from Apache-2.0 (as this gem is) to MIT (as puppet-lint itself is)? Then import the code into puppet-lint.git?

chelnak added a commit to puppetlabs/puppet-lint that referenced this pull request Feb 20, 2023
This commit introduces the top_scope_facts check.

The plugin originated here:

https://github.com/mmckinst/puppet-lint-top_scope_facts-check

and was authored by mmckinst.

Permission has been given to migrate this check to puppet-lints
default plugin set in this PR:

mmckinst/puppet-lint-top_scope_facts-check#11
@seanmil
Copy link
Contributor

seanmil commented Feb 20, 2023

For my part, I'm fine with my contribution being relicensed as suggested. Thanks!

chelnak added a commit to puppetlabs/puppet-lint that referenced this pull request Feb 20, 2023
This commit introduces the top_scope_facts check.

The plugin originated here:

https://github.com/mmckinst/puppet-lint-top_scope_facts-check

and was authored by mmckinst.

Permission has been given to migrate this check to puppet-lints
default plugin set in this PR:

mmckinst/puppet-lint-top_scope_facts-check#11
chelnak added a commit to puppetlabs/puppet-lint that referenced this pull request Feb 21, 2023
This commit introduces the top_scope_facts check.

The plugin originated here:

https://github.com/mmckinst/puppet-lint-top_scope_facts-check

and was authored by mmckinst.

Permission has been given to migrate this check to puppet-lints
default plugin set in this PR:

mmckinst/puppet-lint-top_scope_facts-check#11
chelnak added a commit to puppetlabs/puppet-lint that referenced this pull request Feb 23, 2023
This commit introduces the top_scope_facts check.

The plugin originated here:

https://github.com/mmckinst/puppet-lint-top_scope_facts-check

and was authored by mmckinst.

Permission has been given to migrate this check to puppet-lints
default plugin set in this PR:

mmckinst/puppet-lint-top_scope_facts-check#11
chelnak added a commit to puppetlabs/puppet-lint that referenced this pull request Feb 23, 2023
This commit introduces the top_scope_facts check.

The plugin originated here:

https://github.com/mmckinst/puppet-lint-top_scope_facts-check

and was authored by mmckinst.

Permission has been given to migrate this check to puppet-lints
default plugin set in this PR:

mmckinst/puppet-lint-top_scope_facts-check#11
chelnak added a commit to puppetlabs/puppet-lint that referenced this pull request Feb 23, 2023
This commit introduces the top_scope_facts check.

The plugin originated here:

https://github.com/mmckinst/puppet-lint-top_scope_facts-check

and was authored by mmckinst.

Permission has been given to migrate this check to puppet-lints
default plugin set in this PR:

mmckinst/puppet-lint-top_scope_facts-check#11
chelnak added a commit to puppetlabs/puppet-lint that referenced this pull request Feb 23, 2023
This commit introduces the top_scope_facts check.

The plugin originated here:

https://github.com/mmckinst/puppet-lint-top_scope_facts-check

and was authored by @mmckinst and @seanmil.

Permission has been given to migrate this check to puppet-lints
default plugin set in this PR:

mmckinst/puppet-lint-top_scope_facts-check#11
chelnak added a commit to puppetlabs/puppet-lint that referenced this pull request Feb 23, 2023
This commit introduces the top_scope_facts check.

The plugin originated here:

https://github.com/mmckinst/puppet-lint-top_scope_facts-check

and was authored by @mmckinst and @seanmil.

Permission has been given to migrate this check to puppet-lints
default plugin set in this PR:

mmckinst/puppet-lint-top_scope_facts-check#11
@bastelfreak
Copy link

puppetlabs/puppet-lint#85 this PR now made it into puppet-lint

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.

plugin converts scoped variables into $facts hash plugin should recognize settings top scope variable
7 participants