Closed dagnello closed 8 years ago
hi @jrperritt, this PR is ready for review.
Thanks for the great work @dagnello. I've left some feedback.
@jamiehannaford Thank you for the feedback!
@dagnello To aid in reviewing this, can you post (either in this PR or the issue) links to the files in the source code you're using to define these APIs?
@jrperritt, API doc link added to this PR.
@dagnello right, we'll want links to the API (preferably schema if one exists) in the actual source code since, as you noted, the docs and source often don't align.
@jrperritt ok, add the link in the doc.go for the lbaas_v2 package?
Yeah, that would fine. Just the link(s) where someone can look and see that, say, a certain field exists and is the correct type.
got it, thanks
@jamiehannaford I have addressed your feedback, PR has been updated.
@jrperritt @jamiehannaford ready for review
@jamiehannaford @jrperritt this PR is ready for additional review.
@anguslees can you also review this PR
lgtm from a purely golang pov. I haven't compared against actual LBaaS API at all.
@anguslees thank you for the review
@jamiehannaford @jrperritt I have addressed all comments, please review and merge if ready.
@jrperritt do we look good for a merge? This is now blocking some Kubernetes and Terraform work.
do we look good for a merge?
Definitely not. At the least there's this: https://github.com/rackspace/gophercloud/pull/575#discussion_r64760778 . Then someone (probably me) will need to review it thoroughly by going through the OpenStack source code to validate the API.
This is now blocking some Kubernetes and Terraform work.
The considerations for merging this PR (and indeed all PRs) are its correctness and consistency with the rest of the library. Larger PRs often take more time to go through the review process
@jamiehannaford @jrperritt the remaining comments have been addressed.
@jrperritt Please review, and let us know of any outstanding issues.
All tests pass, merging. Thanks for the great work @dagnello 🚀
@jamiehannaford thank you!
This PR includes LBaaS V2 Support for the following components (ref. #460):
LbaaS v2 API documentation: http://developer.openstack.org/api-ref-networking-v2-ext.html Note: There are some details missing/invalid in current docs, I have been using client calls with --debug flag to see what the API actually returns.