(SDK-232) Add operatingsystem_support defaults #58
Conversation
DavidS
force-pushed
the
sdk-232-operatingsystem-support
branch
8 times, most recently
from
ed51756 to
664939a
Compare
| git_path = ENV['PDK_USE_SYSTEM_BINARIES'].nil? ? File.join(git_bindir, 'git') : 'git' | ||
| vendored_bin_path = File.join(git_bindir, 'git') | ||
| git_path = File.exists?(vendored_bin_path) ? vendored_bin_path : 'git' | ||
| PDK.logger.debug(_("Using git from the system PATH, instead of '%{vendored_bin_path}'") % { vendored_bin_path: vendored_bin_path}) |
There was a problem hiding this comment.
@DavidS I know you mentioned the possibility of adding some sort of "don't fall back to system PATH" option for testing but maybe it would be sufficient to assert that the above DEBUG message isn't logged? No big deal either way.
| "operatingsystem" => "Windows", | ||
| "operatingsystemrelease" => [ "2016" ] | ||
| }, | ||
| ] |
There was a problem hiding this comment.
Do we want to be having all these operating systems set by default or interview for them?
There was a problem hiding this comment.
They are easily discoverable, as this is used by rspec-puppet-facts. Once they are in here, they are easy to change. Interviewing for them sounds like more hassle than it's worth.
|
I didn't try actually running them yet but sounded like @james-stocks was going to try that. Code changes look good. |
Instead of relying on user configuration, this change makes an automatic fallback to the system PATH, if the vendored binary doesn't exist.
These are necessary to make rspec-puppet-facts work, and it is easier for the developer to delete/modify what we get wrong there, than add a new key. This also adds very basic acceptance tests for the `new module` command.
DavidS
force-pushed
the
sdk-232-operatingsystem-support
branch
from
664939a to
ca27451
Compare
|
moved resolved pending example (via #63) to the normal test |
There was a problem hiding this comment.
LGTM
The acceptance:package task fails for me but I believe that's because I'm using an outdated package - the errors I get indicate the acceptance spec is doing the right thing.
These tests also fail for me currently; but I believe it's an issue on my machine - I see these passing on appveyor and travis
|
✨ |
DavidS
changed the title
No description provided.