Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Only set tasks = true when parsing plans. #266

Merged
merged 3 commits into from
May 17, 2021
Merged

Conversation

binford2k
Copy link
Contributor

@binford2k binford2k commented Dec 17, 2020

Only set tasks = true when parsing plans. The tests fail because the tested pathname is not the same pattern, so the regex fails. Do you have suggestions @scotje?

Fixes #251

@binford2k binford2k requested a review from a team December 17, 2020 16:56
@binford2k binford2k changed the title Only set tasks = true when parsing plans. The tests fail because the Only set tasks = true when parsing plans. Dec 17, 2020
The tests fail because the tested pathname is not the same pattern, so
the regex fails. Do you have suggestions @scotje?

Fixes puppetlabs#251
if Puppet::Util::Package.versioncmp(Puppet.version, "5.0.0") < 0 && @file.to_s.match(/^plans\//)
log.warn "Skipping #{@file}: Puppet Plans require Puppet 5 or greater."
return
if @file.to_s.match(/^plans\//)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@scotje this regex fails during testing because it's in the full fixtures directory. If I remove the start-of-string anchor, tests pass, but that's probably not a good solution. Any other ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably extract this test into a helper method and then we can stub that out in the tests

@pmcmaw
Copy link
Contributor

pmcmaw commented Jan 11, 2021

Hey @binford2k just wondering if this is still functionality you are still interested in working on?

@binford2k
Copy link
Contributor Author

@pmcmaw I'll finish it up next week after kickoff

@da-ar
Copy link
Contributor

da-ar commented Mar 29, 2021

Hello @binford2k, there's probably a little more required now to get the PR across the line. What would you like to do?

@da-ar da-ar requested a review from a team as a code owner May 17, 2021 11:47
@da-ar da-ar merged commit 3d06678 into puppetlabs:main May 17, 2021
@chelnak chelnak added the bugfix label Oct 13, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

class with "apply" attribute causes parser error
5 participants