Skip to content

Only set tasks = true when parsing plans. #266

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

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
@da-ar da-ar added feature and removed feature labels May 17, 2021
@chelnak chelnak added the bugfix label Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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