-
Notifications
You must be signed in to change notification settings - Fork 52
Conversation
6b29880
to
a742dd2
Compare
a742dd2
to
5538840
Compare
5538840
to
0d35dd6
Compare
501632d
to
baf9835
Compare
Rakefile: | ||
default_disabled_lint_checks: | ||
- 'relative' | ||
- 'disable_80chars' | ||
- 'disable_class_inherits_from_params_class' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not repeat the defaults from https://github.com/puppetlabs/puppetlabs_spec_helper/blob/master/lib/puppetlabs_spec_helper/rake_tasks.rb#L428-L434
<% checks = @configs['default_disabled_lint_checks'] + ( @configs['extra_disabled_lint_checks'] || [] ) -%> | ||
<% checks.each do |check| -%> | ||
PuppetLint.configuration.send('<%= check %>') | ||
<% end -%> | ||
PuppetLint.configuration.ignore_paths = ["spec/**/*.pp", "pkg/**/*.pp", "vendor/**/*.pp"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not override the comprehensive defaults from https://github.com/puppetlabs/puppetlabs_spec_helper/blob/master/lib/puppetlabs_spec_helper/rake_tasks.rb#L435-L442
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right. I was wondering where these were coming from.
on_build_failure: false | ||
on_build_status_changed: false | ||
<% else -%> | ||
# Appveyor is only enabled on modules that support windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having a broken appveyor file will cause issues when trying to enable appveyor as the "Only run on branches with appveyor.yml" setting will not trigger. See the results on puppetlabs/puppetlabs-stdlib#712
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition of supported OSs (linux and/or windows) is not something built-in to modulesync so it needs ERB templating to present different yaml entries. config_defaults.yml
defines the files to be managed, one of which is appveyor.yml
. It is not possible to conditionalize the management of appveyor.yml
, only define the defaults. And the template itself cannot afaik say "actually I should not be created" only "I should be empty".
Options:
appveyor.yml
exists in all repos and we test on all modules (what is testing apt in appveyor supposed to do? This doesn't make sense)appveyor.yml
key only exists in$module/.sync.yml
and we have the defaults in:global
inconfig_defaults.yml
(this is an ugly hack)- modulesync is expanded to have templates be conditionally managed (complex, low return, and don't want to wait for this)
appveyor.yml
exists in all repos and we manually enable/disable testing (one time cost per module, imho easiest option)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only appears to happen on apt and stdlib?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't currently have the tests setup to do so, but long-term testing should cover whether facts and other native code can be plugin-synced to all (agent-)supported platforms and not break a run when none of the dependencies are installed.
For some modules, like stdlib, we should already be testing everything on windows, as it is a supported platform.
or, 5. manually set appveyor.yml
to unmanaged: true
in the local .sync.yml
.
apt and stdlib are the only ones where I started playing around with appveyor.
🎉 so excited to see this happening! |
rainbow pin? |
appveyor.yml: | ||
appveyor_bundle_install: "bundle install --jobs 4 --retry 2 --without system_tests" | ||
matrix: | ||
- PUPPET_GEM_VERSION: '~> 3.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
puppet 3?
2d63d60
to
55f5ab1
Compare
55f5ab1
to
3012627
Compare
(MODULES-4098) Sync more files see note on #125
No description provided.