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

feat: code parity with other native providers #11

Merged

Conversation

frezbo
Copy link
Contributor

@frezbo frezbo commented Aug 4, 2021

Fixes #9 and #3

Signed-off-by: Noel Georgi [email protected]

Copy link
Member

@mikhailshilkov mikhailshilkov left a comment

Choose a reason for hiding this comment

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

This looks great, thank you for taking the time to refactor this! A few small comments below.

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@frezbo frezbo force-pushed the feature/parity-with-native-provider branch from 95ca3db to 8173048 Compare August 13, 2021 15:22
@frezbo
Copy link
Contributor Author

frezbo commented Aug 13, 2021

@mikhailshilkov fixed the review comments. Thanks for taking a look 🤗

Copy link
Member

@mikhailshilkov mikhailshilkov left a comment

Choose a reason for hiding this comment

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

LGTM. @viveklak do you want to take I look before I merge?

@lukehoban
Copy link

BTW - @stack72 - there are a number of places where the Makefile here differs from what we have in all our other boilerplates. It's probably a future follow-up thing, not something we need to block this PR on, but it would be good to rationalize across the boilerplate repos (for both native providers and components) since both should in principle be effectively "the same" from a Makefile perspective. We should likely then push that concsisten approach back up into some of the 1st party native providers so we have a consistent approach to how providers are built/published.

Comparison: https://github.com/pulumi/pulumi-component-provider-go-boilerplate/blob/main/Makefile

@lukehoban
Copy link

I think this is good to merge now as a first step. @viveklak or @stack72 anything you wanted to review before landing this? I do expect we may want some follow-up to standardize a little more between the component and native provider boilerplates, but that can easily be follow up.

@frezbo
Copy link
Contributor Author

frezbo commented Aug 25, 2021

@lukehoban oops I missed the comments. I was mostly basing the makefile from the native providers. I forgot to look at the ones in the component providers. That could be done as a separate PR

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

Successfully merging this pull request may close these issues.

Update structure to mirror azure/kubernetes repos
3 participants