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

PDK Convert & Some tests passing on Puppet 6 #84

Merged
merged 2 commits into from
Sep 10, 2020
Merged

PDK Convert & Some tests passing on Puppet 6 #84

merged 2 commits into from
Sep 10, 2020

Conversation

frizop
Copy link
Collaborator

@frizop frizop commented Sep 4, 2020

No description provided.

@jhoblitt
Copy link
Owner

jhoblitt commented Sep 8, 2020

I really appreciate the effort that's going into this PR. Could someone explicitly ping me when it is ready for merge in case I miss the title being updated?

@frizop
Copy link
Collaborator Author

frizop commented Sep 9, 2020

@jhoblitt no problem, just pinging you now that all tests pass! Quite welcome on the effort.

EDIT: Solved the older EL5 version issues

@frizop frizop changed the title [WIP] PDK Convert & Some tests passing on Puppet 6 PDK Convert & Some tests passing on Puppet 6 Sep 9, 2020
@frizop
Copy link
Collaborator Author

frizop commented Sep 9, 2020

Alright, I think that's it. @jhoblitt I did end up doing some more to the module to clean things up like added @param options to the class docs. I accidentally wiped out your previous commit/PR (and then added the stuff back in) as follows:

commit 9460bdc70bfb4ca831d7fe57154fcb73c792fb08
Author: Rike-Benjamin Schuppner <rikebs@debilski.de>

So I just pushed up a cherry-pick to make sure that it is contained in the repo. Apologies for that.

@jhoblitt
Copy link
Owner

jhoblitt commented Sep 9, 2020

I would be fine with making a major version bump and dropping anything older than rhel/centos 7.

@jhoblitt
Copy link
Owner

jhoblitt commented Sep 9, 2020

@frizop I'll take a detailed look once CI passes. Could I ask you to do a rebase to remove the merge commit?

@frizop
Copy link
Collaborator Author

frizop commented Sep 9, 2020

@frizop I'll take a detailed look once CI passes. Could I ask you to do a rebase to remove the merge commit?

Sure thing. Give me a minute and I'll push that up

@frizop
Copy link
Collaborator Author

frizop commented Sep 9, 2020

Sadly I had to force push again because that merge got involved. Let me know if this looks good for you @jhoblitt
EDIT: cry more tests are failing now...

- pdk validate -a run after all code changes to verify (and it fixed a bunch of stuff)
- accidentally removed travis (which is needed for our external testing)
- type: :rvalue was required for functions that return data
- Re-added EL5 support, tests also pass now

There's for sure a good reason to upgrade functions to a newer puppet
compatibility version https://puppet.com/docs/puppet/5.5/functions_ruby_overview.html

Fixup for metadata.json
@jhoblitt
Copy link
Owner

This looks great. Thank you for putting all the effort into this!

@jhoblitt jhoblitt merged commit 1702610 into jhoblitt:master Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants