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 template sync #550

Closed
wants to merge 15 commits into from
Closed

PDK template sync #550

wants to merge 15 commits into from

Conversation

DavidS
Copy link
Contributor

@DavidS DavidS commented Feb 14, 2019

These change bring the modulesync_config templates and setup closer to the current pdk-templates. Merging these rather pedestrian changes will be a first step towards completely switching over to PDK and PDKsync.

Splitting the easy changes out like this, clears the path to investigating the remaining more complicated changes.

See https://github.com/DavidS/pdk-templates/tree/pdk-template-sync for more WIP on the other side.

For now, please pick apart the changes here, to make sure that I'm following your preferences, and haven't bolloxed up something. i've only tested this against a single module and have not even run tests or validation over that yet.

Copy link
Member

@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.

Overall 👍 for moving it closer to PDK.

moduleroot/Rakefile.erb Outdated Show resolved Hide resolved
moduleroot/Rakefile.erb Outdated Show resolved Hide resolved
moduleroot/spec/default_facts.yml.erb Outdated Show resolved Hide resolved
moduleroot/spec/spec_helper.rb.erb Outdated Show resolved Hide resolved
@ekohl
Copy link
Member

ekohl commented Apr 6, 2019

@DavidS we've done some cleanups in master. Would you mind rebasing this?

@DavidS
Copy link
Contributor Author

DavidS commented Apr 8, 2019

This first force-push was only to work in ekohl's comments and make sure that the code as it was two weeks ago is fine. I'll do the full rebase next. wish me luck :-)

DavidS added 15 commits April 8, 2019 20:56
Please give a shout if one of those is used extensively across VP
contributors.
For different usages this needs to change, so we can't override it here
The PDK team is maintaining meta-gems (only containing dependencies)
for a number of core gems used by the PDK and the community at large.
See https://github.com/puppetlabs/puppet-module-gems for details

This commit switches to the pdk-templates core group/gem loop, updates
the location_for method, and adjusts the default dependencies to
use the puppet-module-gems.
This commit contains all outstanding changes that were not contained
in the last commit
This also adds a small warning message, if the gem can't be found.
While this is unlikely to be hit by modulesynced modules, it'll be
a lifesaver that one time when something goes awry.

Please don't ask why I know ;-)
@DavidS DavidS changed the title (WIP) PDK template sync PDK template sync Apr 8, 2019
@DavidS
Copy link
Contributor Author

DavidS commented Apr 8, 2019

This now contains the rebase on top of 2.7.0. As feared the compare page is a lot less helpful to understand the diff :-/

Unless I broke something or someone else finds fault in the changes here, I think this is good to go in as a first step.

@ekohl
Copy link
Member

ekohl commented Apr 8, 2019

I toyed with a rebase to see if I could use our modulesync repo as a PDK template: https://github.com/ekohl/modulesync_config/commits/pdk-style. PDK barfed for me on bundler after correctly syncing.

Overall I'm still not convinced about PDK. I really hate that it puts the PDK version in metadata.json which IMHO only adds noise. It also downgrades various gems while pulling in a lot more than I'd have a need for. I won't hold back using PDK in Voxpupuli but I don't see myself using it any time soon.

That said, I still like the goal of a very minimal set of synced files. Reading the git log and only seeing noise in modulesynced files is very distracting.

@@ -71,8 +71,7 @@ task :reference, [:debug, :backtrace] do |t, args|
Rake::Task['strings:generate:reference'].invoke(patterns, args[:debug], args[:backtrace])
end

begin
require 'github_changelog_generator/task'
if Bundler.rubygems.find_name('github_changelog_generator').any?
Copy link
Member

Choose a reason for hiding this comment

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

How about if defined? GitHubChangelogGenerator so you don't rely on bundler?


def ensure_module_defined(module_name)
Copy link
Member

Choose a reason for hiding this comment

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

Should this live in puppetlabs_spec_helper instead?

@ekohl
Copy link
Member

ekohl commented Jul 24, 2020

@DavidS any plans to continue this effort or should we close it?

@ekohl
Copy link
Member

ekohl commented Nov 6, 2020

Given the lack of activity and that we've since introduced our own voxpupuli-test and voxpupuli-acceptance gems I'm closing this now.

@ekohl ekohl closed this Nov 6, 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.

3 participants