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-386) Remove parameter options from 'new class' #236

Merged
merged 1 commit into from
Aug 11, 2017
Merged

(PDK-386) Remove parameter options from 'new class' #236

merged 1 commit into from
Aug 11, 2017

Conversation

austb
Copy link
Contributor

@austb austb commented Aug 9, 2017

No description provided.

@puppetlabs puppetlabs deleted a comment from coveralls Aug 10, 2017
scotje
scotje approved these changes Aug 10, 2017
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.

Looks good. Coveralls claims the coverage went down but can't actually show any lines that no longer have coverage as a result, so I say we can just merge.

@rodjek
Copy link
Contributor

rodjek commented Aug 11, 2017

Rebased and modified the commit to remove a few more mentions of the class parameters from the docs.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 92.223% when pulling f4c59a4 on austb:pdk-386 into 104ab4b on puppetlabs:master.

@puppetlabs puppetlabs deleted a comment from coveralls Aug 11, 2017
rodjek
rodjek approved these changes Aug 11, 2017
View reviewed changes
Copy link
Contributor

@rodjek rodjek left a comment

Choose a reason for hiding this comment

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

I agree with @scotje that the coverage change failure is meaningless in this case, 👍

@rodjek rodjek merged commit 5f97a53 into puppetlabs:master Aug 11, 2017
@DavidS DavidS added the bug label Aug 11, 2017
@austb austb deleted the pdk-386 branch August 11, 2017 15:55
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

@rodjek rodjek rodjek approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@austb @rodjek @coveralls @scotje @DavidS