Skip to content

Allow convert_file_resources to work even in absence of environments/production#1

Merged
kpaulisse merged 5 commits intomasterfrom
kpaulisse-file-compare-environments-production
Oct 20, 2016
Merged

Allow convert_file_resources to work even in absence of environments/production#1
kpaulisse merged 5 commits intomasterfrom
kpaulisse-file-compare-environments-production

Conversation

@kpaulisse
Copy link
Copy Markdown
Contributor

@kpaulisse kpaulisse commented Oct 19, 2016

Overview

This pull request avoids an exception being thrown when:

  • Catalog caching is enabled
  • The "to" branch is .
  • The environments/production symlink is missing

The changed code checks additional paths, and skips the code that will be problematic when the Puppet repository is not set up as expected.

Checklist

  • Make sure that all of the tests pass, and fix any that don't. Just run rake in your checkout directory, or review the CI job triggered whenever you push to a pull request.
  • Make sure that there is 100% test coverage by running rake coverage:spec or ignoring untestable sections of code with # :nocov comments. If you need help getting to 100% coverage please ask; however, don't just submit code with no tests.

Copy link
Copy Markdown

@ross ross left a comment

Choose a reason for hiding this comment

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

Seems reasonable. One small questions/suggestion about a comment inline.

# there is on-demand, explicit error checking for each file within the modification loop.
return unless compilation_dir.is_a?(String) && compilation_dir != ''

# Making sure that compilation_dir/environments/production exists. Otherwise, try to find
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like this is actually making sure that <compilation_dir>/environments/production/modules exists. The part below includes the /modules. Unless it's intention, they should probably match up.

@kpaulisse kpaulisse merged commit 34208d4 into master Oct 20, 2016
@kpaulisse kpaulisse deleted the kpaulisse-file-compare-environments-production branch October 20, 2016 00:25
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