-
Notifications
You must be signed in to change notification settings - Fork 27
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
Support simple resource class selection #7
Conversation
crossplane/crossplane#926 crossplane/crossplane#927 crossplane/crossplane-runtime#48 Updates angryjet to reflect the above changes. Signed-off-by: Nic Cope <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would you feel about incorporating #2 into this PR? Just thinking that if we are going to bump stack dependencies that it might be nice to only have to do so once. Otherwise, LGTM!
I agree with the sentiment, but I'm inclined to tackle it in a separate PR because it's a slightly less trivial change than it might seem. angryjet doesn't know how to write anything but methods at the moment, so we'd probably have to teach |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/LGTM
@negz FYI, my changes also have a change in the api (adding |
@soorena776 Thanks for the heads up. My suspicion is it might be best to add your changes in a separate PR, because it might take me a while to merge this one. |
@negz sounds good! |
crossplane/crossplane#926
crossplane/crossplane#927
crossplane/crossplane-runtime#48
Updates angryjet to reflect the above changes.
Description of your changes
Checklist
I have:
make reviewable
to ensure this PR is ready for review.