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

Remove deprecated hiera and validation functions #119

Merged
merged 2 commits into from
Mar 12, 2018

Conversation

jkroepke
Copy link
Contributor

@jkroepke jkroepke commented Feb 20, 2017

Related #118

  • Replace deprecated hiera functions with newer lookup functions
  • Replace stdlib's validation functions with puppet data types (bump stdlib minimum version to 4.13)
  • Removed unused fact gitlab_systemd
  • Added Dockerfile to run rspec tests locally without setup a dev environment.

@jkroepke
Copy link
Contributor Author

Need help here, I don't know why the yaml is generated wrong.

@tobru tobru added needs-help Extra attention is needed puppet4 labels Mar 15, 2017
@@ -83,15 +83,15 @@
if $config_manage {
if $source_config_file {
file { $config_file:
ensure => file,
Copy link
Member

Choose a reason for hiding this comment

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

why change this to present, file is much more descriptive and does the same thing (i thought puppet-lint flagged this but dont see it on their we site)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know why I'm doing this the last six month. I will change it back.

@b4ldr
Copy link
Member

b4ldr commented Jul 11, 2017

can you give more info on the issue, happy to help with this

@jkroepke
Copy link
Contributor Author

I'll take a rebase and rollback the changes on the ensure attributes.

@jkroepke jkroepke force-pushed the puppet4 branch 8 times, most recently from 7c2af9a to 8e7320b Compare July 13, 2017 23:23
@jkroepke jkroepke force-pushed the puppet4 branch 6 times, most recently from 8d2857d to 45852ba Compare March 8, 2018 01:19
@jkroepke jkroepke changed the title [WIP] Remove deprecated hiera and validation functions Remove deprecated hiera and validation functions Mar 8, 2018
@jkroepke jkroepke force-pushed the puppet4 branch 6 times, most recently from 43e9a43 to 81d7cc2 Compare March 8, 2018 02:07
@jkroepke jkroepke force-pushed the puppet4 branch 2 times, most recently from a531740 to 2be5d16 Compare March 8, 2018 02:31
@jkroepke
Copy link
Contributor Author

jkroepke commented Mar 8, 2018

Sorry for some possible notification spam.

@tobru @bastelfreak

Any chances to get merged this PR? Rebasing this PR everytime is a hard doing.

Copy link
Contributor

@LongLiveCHIEF LongLiveCHIEF left a comment

Choose a reason for hiding this comment

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

Can you switch the legacy fact and the puppet version requirement semver and then rebase? I'll watch for these changes and merge when they come in so you won't have to rebase this one again (after this last time).

comment => 'GitlabCI Runner Repo',
location => "${repo_base_url}/runner/${package_name}/${distid}/",
release => $::lsbdistcodename,
location => "${repo_base_url}/runner/${package_name}/${::lsbdistid.downcase}/",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a legacy fact. Can you switch it to a modern fact please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you want to fix the sped tests, sure. I tried to used the modern facts, but the lib rspec-puppet-facts doesn‘t provide modern facts and I get tons of error in travis.

Copy link
Contributor

Choose a reason for hiding this comment

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

nah, sounds like out of scope for this PR in that case. We'll fix it in another issue/PR.


if ! ($content) and ! ($source) {
fail('gitlab::custom_hook resource must specify either content or source')
fail("gitlab::custom_hook[${name}]: Must specify either content or source")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice touch 👍

metadata.json Outdated
@@ -68,7 +68,7 @@
"requirements": [
{
"name": "puppet",
"version_requirement": ">=3.7.0 <5.0.0"
"version_requirement": ">=4.6.1 <5.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't the version_requirement here now be ">=4.6.1 <6.0.0 instead of 5.0.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I created this PR so long ago.

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel you. I've got a few of my own that have had to be rebased a handful of times.

@jkroepke
Copy link
Contributor Author

So I fixed the lint issues from master, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-help Extra attention is needed needs-rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants