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

(CONT-339) Add top scope facts check #85

Merged
merged 1 commit into from
Feb 23, 2023

Conversation

chelnak
Copy link

@chelnak chelnak commented 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 and @seanmil.

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

mmckinst/puppet-lint-top_scope_facts-check#11

@chelnak chelnak requested a review from a team as a code owner February 20, 2023 16:43
@chelnak chelnak self-assigned this Feb 20, 2023
@chelnak chelnak added the enhancement New feature or request label Feb 20, 2023
@chelnak chelnak force-pushed the CONT-339-add_top_scope_facts_check branch from 7ed7bf8 to b4d4b5c Compare February 20, 2023 16:46
puppet-lint.gemspec Outdated Show resolved Hide resolved
@chelnak chelnak force-pushed the CONT-339-add_top_scope_facts_check branch from b4d4b5c to 41f69d3 Compare February 20, 2023 16:57
@chelnak chelnak changed the title Cont 339 add top scope facts check (CONT-339) Add top scope facts check Feb 20, 2023
@chelnak chelnak force-pushed the CONT-339-add_top_scope_facts_check branch from 41f69d3 to 95bc4ff Compare February 21, 2023 09:30
Copy link

@GSPatton GSPatton left a comment

Choose a reason for hiding this comment

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

tested locally and works as expected.

Copy link

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Should the commit also credit the other contributor?

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@chelnak
Copy link
Author

chelnak commented Feb 21, 2023

Yes good point. I can get that updated.

With regards to your other comments:

  • I'm leaning towards moving the min ruby change out of this PR

  • If we do the above then I can also move the CI changes out too

  • Ruby 3.x is on our radar. I think the team will be jumping on this and puppet 8 testing very soon.

@chelnak chelnak force-pushed the CONT-339-add_top_scope_facts_check branch 4 times, most recently from 7b1f025 to f0cd1ec Compare February 23, 2023 15:24
@chelnak chelnak force-pushed the CONT-339-add_top_scope_facts_check branch from f0cd1ec to 77b7e62 Compare February 23, 2023 15:40
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 chelnak force-pushed the CONT-339-add_top_scope_facts_check branch from 77b7e62 to d416062 Compare February 23, 2023 15:44
@chelnak chelnak requested review from bastelfreak and ekohl and removed request for ekohl February 23, 2023 15:48
@alexjfisher
Copy link

alexjfisher commented Feb 24, 2023

Should the commit also credit the other contributor?

But not me? ;)

(I'm happy for my changes from mmckinst/puppet-lint-top_scope_facts-check#11 to be licensed as either MIT or Apache-2)

@chelnak
Copy link
Author

chelnak commented Feb 24, 2023

Oh no! Sorry @alexjfisher. My bad. I can get this updated for you today.

bastelfreak added a commit to bastelfreak/voxpupuli-puppet-lint-plugins that referenced this pull request Feb 24, 2023
puppet-lint-top_scope_facts-check got vendored into puppet-lint 3:
puppetlabs/puppet-lint#85
We need to wait with merging this until puppet-lint 3.1 is released.
This also depends on mmckinst/puppet-lint-legacy_facts-check#41
@alexjfisher
Copy link

Oh no! Sorry @alexjfisher. My bad. I can get this updated for you today.

Nah, don't worry about it. It's in the merged commit message so what were you planning on updating anyway? :)

@chelnak
Copy link
Author

chelnak commented Feb 24, 2023

#90 Adds you to the checks description 👍

bastelfreak added a commit to bastelfreak/voxpupuli-puppet-lint-plugins that referenced this pull request Feb 28, 2023
puppet-lint-top_scope_facts-check got vendored into puppet-lint 3:
puppetlabs/puppet-lint#85
We need to wait with merging this until puppet-lint 3.1 is released.
This also depends on mmckinst/puppet-lint-legacy_facts-check#41
bastelfreak added a commit to bastelfreak/voxpupuli-puppet-lint-plugins that referenced this pull request Feb 28, 2023
puppet-lint-top_scope_facts-check got vendored into puppet-lint 3.1:
puppetlabs/puppet-lint#85

puppet-lint-legacy_facts-check got vendored into puppet-lint 3.1:
puppetlabs/puppet-lint#91
bastelfreak added a commit to bastelfreak/voxpupuli-puppet-lint-plugins that referenced this pull request Feb 28, 2023
puppet-lint-top_scope_facts-check got vendored into puppet-lint 3.1:
puppetlabs/puppet-lint#85

puppet-lint-legacy_facts-check got vendored into puppet-lint 3.1:
puppetlabs/puppet-lint#91
bastelfreak added a commit to bastelfreak/voxpupuli-puppet-lint-plugins that referenced this pull request Feb 28, 2023
puppet-lint-top_scope_facts-check got vendored into puppet-lint 3.1:
puppetlabs/puppet-lint#85

puppet-lint-legacy_facts-check got vendored into puppet-lint 3.1:
puppetlabs/puppet-lint#91
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.

6 participants