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

Libcloud 574 routes and route tables #313

Closed
wants to merge 16 commits into from
Closed

Libcloud 574 routes and route tables #313

wants to merge 16 commits into from

Conversation

@zerthimon
Copy link
Contributor

@zerthimon zerthimon commented Jun 10, 2014

Support for EC2 Route Tables and Routes.

@zerthimon
Copy link
Contributor Author

@zerthimon zerthimon commented Jun 10, 2014

@Kami is there anything else I should do here ?

Loading

:type id: ``str``
:param routes: A list of routes in the route table.
:type routes: ``list`` of :class:`EC2Route`
Copy link
Member

@Kami Kami Jun 11, 2014

Choose a reason for hiding this comment

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

Please add . before the classes which are located in the same module. In this case:

 :type       routes: ``list`` of :class:`.EC2Route`

Loading

@Kami
Copy link
Member

@Kami Kami commented Jun 11, 2014

Ok, reviewed the whole pull request.

There are some minor issues left, but it mostly looks good. Next step would be adding tests for this new functionality. Let us know if you need help with that.

Loading

@Kami
Copy link
Member

@Kami Kami commented Jun 12, 2014

@zerthimon Let me know once those issues have been addressed and I will get it merged into trunk.

Loading

@zerthimon
Copy link
Contributor Author

@zerthimon zerthimon commented Jun 16, 2014

@Kami

Fixed issues that you found during code review.
Also changed ex_create_route() and ex_replace_route() to accept objects and extract IDs, instead of passing IDs as strings.

Loading

@asfgit asfgit closed this in fcd353d Jun 16, 2014
@Kami
Copy link
Member

@Kami Kami commented Jun 16, 2014

Merged into trunk. Thanks.

Loading

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

Successfully merging this pull request may close these issues.

None yet

2 participants