Skip to content
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.

Sign up for GitHub

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

Jump to bottom

(PDK-1047) Add --add-tests to pdk convert #746

Merged
merged 2 commits into from
Sep 16, 2019
Merged

(PDK-1047) Add --add-tests to pdk convert #746

merged 2 commits into from
Sep 16, 2019

Conversation

rodjek
Copy link
Contributor

@rodjek rodjek commented Sep 9, 2019

No description provided.

@rodjek rodjek added the WIP label Sep 9, 2019
@coveralls
Copy link

coveralls commented Sep 9, 2019 •
edited
Loading

Coverage Status

Coverage increased (+0.05%) to 92.035% when pulling aee46fe on rodjek:pdk-1047 into 1389b50 on puppetlabs:master.

@scotje
Copy link
Contributor

scotje commented Sep 10, 2019

After some deliberation we're thinking that pdk convert --add-tests is a good flag for activating this functionality.

@rodjek rodjek force-pushed the pdk-1047 branch 3 times, most recently from 9d429fd to ae9b758 Compare September 11, 2019 05:42
@rodjek rodjek removed the WIP label Sep 11, 2019
@rodjek rodjek changed the title (PDK-1047) Add --generate-missing-tests to pdk convert (PDK-1047) Add --add-tests to pdk convert Sep 11, 2019
scotje
scotje approved these changes Sep 12, 2019
View reviewed changes
Copy link
Contributor

@scotje scotje left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, left a couple small notes.

spec/acceptance/convert_spec.rb Outdated Show resolved Hide resolved
lib/pdk/module/convert.rb
add_tests!
else
PDK::Report.default_target.puts(_('No changes required.'))
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm, this is unfortunate. I see a refactoring in this methods future....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I might be able to clean this up a bit, but theres only so much we can do without an extensive refactor

@rodjek rodjek merged commit 378f22d into puppetlabs:master Sep 16, 2019
@rodjek rodjek deleted the pdk-1047 branch September 16, 2019 03:34
@rodjek rodjek added this to the September 2019 milestone Oct 9, 2019
@rodjek rodjek added the feature label Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scotje scotje scotje approved these changes

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
September 2019
Development

Successfully merging this pull request may close these issues.
3 participants
@rodjek @coveralls @scotje