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.

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

Added completion for resource-like class #180

Merged

Conversation

juliosueiras
Copy link
Contributor

@juliosueiras juliosueiras commented Sep 12, 2019

Fixes #181

Added Completion for resource-like class declare

@jpogran
Copy link
Contributor

jpogran commented Sep 13, 2019

Thanks for the PR! We're in the midst of making the Sept release for this extension, so we'll review this for inclusion in that release. We should have a reply back by next week, if not sooner.

@jpogran jpogran added this to the 0.22.0 milestone Sep 13, 2019
Copy link
Contributor

@jpogran jpogran left a comment

Choose a reason for hiding this comment

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

Overall this looks good. I've tagged @glennsarti as reviewer to put a final pass on this, he'll be back next week.

In the meantime, can you squash your commits and make a meaningful commit message? If you don't know how to squash or can't, I can do that for you, just let me know. As we wrote in the contributing doc, we prefer commit messages that explain the change so when we come back to this commit we can learn about what was changed and why. If that's something you'd prefer not to do, just let me know and I'll update it before merge.

@genebean
Copy link
Contributor

ooooo, very nice.

Modified completion provider to allow completion for resource-like class

link: https://puppet.com/docs/puppet/5.3/lang_classes.html#using-resource-like-declarations

class keyword behave like resource when using a defined class, and name parameter become the class name to lookup by the helper
@juliosueiras juliosueiras force-pushed the features/resource-class-completion branch 2 times, most recently from 01aa770 to 35822b6 Compare September 13, 2019 17:28
@juliosueiras
Copy link
Contributor Author

ok, did a rebase and added more details in the commit message, which part need more details?

This commit adds unit tests for resource like declarations in the completion
provider.

Note that unit tests are unsed instead of integration tests as part of the push
to move many of the integration tests into unit.
This commit refactors the resource-like class detection to reduce duplicated
code and adds more comments on this pseudo resource.
@glennsarti
Copy link
Contributor

Hey @juliosueiras !! Thankyou so much for this PR. I've added some tests and refactored the detection logic to reduce duplication. Just waiting for the CI to go green and I'll merge.

Again, thankyou so much for taking to time to add this 🙏

@glennsarti glennsarti merged commit 95320c2 into puppetlabs:master Sep 16, 2019
@glennsarti glennsarti self-assigned this Sep 16, 2019
@binford2k
Copy link
Contributor

I could be wrong, but this looks like a first time community PR! 🎉

@juliosueiras Puppet would love to send you a small token to thank you and welcome you to our community. Can you send an email to community@puppet.com with your mailing address?

@juliosueiras
Copy link
Contributor Author

sure =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add completion for resource-like declarations
5 participants