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

Digitalocean ex_rename_node() extension method #252

Closed
wants to merge 1 commit into from
Closed

Digitalocean ex_rename_node() extension method #252

wants to merge 1 commit into from

Conversation

fluffybeing
Copy link

@Kami as you have told me; I have added the extension method for renaming digitalocean node and also test suit for it.

@Kami
Copy link
Member

Kami commented Feb 18, 2014

This looks like a good start.

There are some lint and test failure issues though - https://travis-ci.org/apache/libcloud/builds/19144330

For tests to pass you need to fix the method call, add a response fixture and modify the MockHttpClass to return your response fixture on rename node HTTP request.

@@ -127,6 +127,12 @@ def destroy_node(self, node):
params=params)
return res.status == httplib.OK

def ex_rename(self, node, name):
Copy link
Member

Choose a reason for hiding this comment

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

Please add _node suffix to the method name to make it clear that the method operates on a node.

@fluffybeing
Copy link
Author

@Kami Thanks I will make the changes :)

@fluffybeing
Copy link
Author

@Kami done 👍

@Kami
Copy link
Member

Kami commented Feb 19, 2014

@rahulrrixe Looking good.

Please just squash all the commits (https://libcloud.readthedocs.org/en/latest/development.html#squash-the-commits-and-generate-the-patch) and I will merge changes into trunk.

added test suite for the ex_rename() extention method

changed ex_rename() to ex_rename_node() extention method

Added the MockHttptestcase from ex_rename_node() extension method

changed ex_rename() to ex_rename_node() extention method

corrected the MockHttptestcase for ex_rename_node() method

Added a fixture for ex_rename_node() method
@Kami
Copy link
Member

Kami commented Feb 19, 2014

Merged into trunk, thanks.

This pull request can be closed now.

@fluffybeing
Copy link
Author

@Kami Thanks 👍 looking forward for next task.

@asfgit asfgit closed this in d10676a Feb 20, 2014
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.

None yet

2 participants