Skip to content

Enable PDK (validation, testing)#15

Merged
edestecd merged 4 commits intoedestecd:masterfrom
comsolit:feature/enable-pdk
May 10, 2018
Merged

Enable PDK (validation, testing)#15
edestecd merged 4 commits intoedestecd:masterfrom
comsolit:feature/enable-pdk

Conversation

@bittner
Copy link
Copy Markdown
Contributor

@bittner bittner commented Oct 26, 2017

As mentioned in #9, a few changes can make it possible to use Puppet's new PDK with this module.

I've also fixed the linter complaints pdk validate has reported.

@bittner bittner force-pushed the feature/enable-pdk branch 2 times, most recently from f81769c to be1a4b6 Compare December 7, 2017 15:23
@bittner
Copy link
Copy Markdown
Contributor Author

bittner commented Dec 7, 2017

The Travis configuration now uses the PDK commands and the beautiful Travis build stages feature for enhanced readability.

We could specify a list of Rubys or RVMs in addition for running a build matrix, e.g.

rvm:
  - 2.3
  - 2.4

More details in the related docs.

You probably know better what would be sensible to test against.

@bittner
Copy link
Copy Markdown
Contributor Author

bittner commented Dec 11, 2017

Note that PR #16 contains changes that will conflict with this PR.

Specifically, the RSpec tests for Skype require the old hash rocket syntax and no trailing comma after the last item in the with clause. This should could be changed when merging to:

          it {
            is_expected.to contain_apt__source('skype-stable')
              .with(
                location: 'https://repo.skype.com/deb',
                release: 'stable',
                repos: 'main',
                architecture: 'amd64',
              )
          }

@bittner
Copy link
Copy Markdown
Contributor Author

bittner commented Dec 12, 2017

This PR is now ready for merging.

@bittner
Copy link
Copy Markdown
Contributor Author

bittner commented Jan 29, 2018

@edestecd Any chance of getting this merged?

@bittner
Copy link
Copy Markdown
Contributor Author

bittner commented Apr 4, 2018

🔔 ping!

Please merge!

Comment thread .travis.yml
script: pdk validate 2> /dev/null

- stage: test
script: pdk test unit 2> /dev/null
Copy link
Copy Markdown
Contributor Author

@bittner bittner Apr 4, 2018

Choose a reason for hiding this comment

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

The redirection of the progress indicators may not be needed anymore (or be replaced by command line options) for the latest versions of the PDK. But that can also be done once this PR is merged.

@bittner
Copy link
Copy Markdown
Contributor Author

bittner commented May 8, 2018

@edestecd Any chance to get this merged?

@edestecd
Copy link
Copy Markdown
Owner

edestecd commented May 8, 2018

@bittner Sorry, thanks for bugging me about it. This module needs some serious love. I finally got around to learning PDK and converted some of our modules already. Can you use a newer version of PDK and use pdk convert: https://puppet.com/docs/pdk/1.x/pdk_reference.html#pdk-convert-command

PDK version 1.4.1 or 1.5.0 will do. I think my other modules are pinned at 1.4.1, so thats probably best: https://github.com/edestecd/puppet-mariadb/blob/master/metadata.json#L74

Also can you use this .sync.yml? https://github.com/edestecd/puppet-mariadb/blob/master/.sync.yml
Put it in prior to running pdk convert to get our preferred template settings.
I'll update the secure password for puppetforge after its merged.

@bittner
Copy link
Copy Markdown
Contributor Author

bittner commented May 8, 2018

I don't have access to the comsolit repository anymore (stopped working with that client). It would be easier for me if you could merge the PR "as is", and we deal with refactoring things afterwards. I believe the build should still pass.

@edestecd
Copy link
Copy Markdown
Owner

edestecd commented May 8, 2018

I was going to say whats the point, b/c the new PDK will replace all the files you edited....
But you did fix the -> ordering statements in quite a few classes and there is value to that.

I will probably merge this then and add the new PDK convert after.

@edestecd
Copy link
Copy Markdown
Owner

edestecd commented May 8, 2018

@bittner thanks for your contribution and pestering me to get this in.

@edestecd
Copy link
Copy Markdown
Owner

edestecd commented May 8, 2018

If you want to do the PDK convert also, let me know and i'll wait

@bittner
Copy link
Copy Markdown
Contributor Author

bittner commented May 8, 2018

If you want to do the PDK convert also, let me know and i'll wait

No, go on with the merging. Thank you!

@edestecd edestecd merged commit d09468f into edestecd:master May 10, 2018
@bittner bittner deleted the feature/enable-pdk branch May 10, 2018 18:18
@bittner
Copy link
Copy Markdown
Contributor Author

bittner commented May 10, 2018

Thank you! 👍

@bittner
Copy link
Copy Markdown
Contributor Author

bittner commented May 10, 2018

The build on master fails now related to Windows tooling, if I'm not totally mistaken:

Could not find class ::chocolatey

Not sure what's going on. 😕

@edestecd
Copy link
Copy Markdown
Owner

I'll look in a bit. I'm going to update pdk in a few.

@bittner
Copy link
Copy Markdown
Contributor Author

bittner commented May 10, 2018

Could it be that chocolately needs to be added to the .fixtures.yml?

@edestecd
Copy link
Copy Markdown
Owner

more than likely

@edestecd
Copy link
Copy Markdown
Owner

Whew, there were a lot of windows issues I had to fix. I suspect the tests were not actually running against windows before. Probably the facts gem did not contain facts for the versions of windows specified in out metadata.json. Anyways its all fixed up now.

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.

2 participants