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

Add missing api for cinder #791

Merged
merged 3 commits into from
Sep 13, 2016
Merged

Conversation

biogerm
Copy link

@biogerm biogerm commented Aug 31, 2016

This change was originally committed by Elina Meier (elina.meier@ericsson.com).
The commit was modified and adapted to openstack4j 3.x by Qin An

In this commit, VolumeType was supported.

This change was originally committed by Elina Meier (elina.meier@ericsson.com).
The commit was modified and adapted to openstack4j 3.x by Qin An
@vinodborole
Copy link
Contributor

Thanks @biogerm for the contribution, looks good. It would be good if you can add few units test around this?

@biogerm
Copy link
Author

biogerm commented Sep 1, 2016

@vinodborole I actually removed the tests which are wrote based on openstack4j 1.x
I will see what I can do

@auhlig
Copy link
Member

auhlig commented Sep 1, 2016

Any reason for changing the import pattern in the Builders?

@biogerm
Copy link
Author

biogerm commented Sep 5, 2016

@auhlig Thanks for pointing out. I guess it was because of the merge between really old code. And I realized that the other comment I was suppose to sent was not sent... Here it goes...

@biogerm
Copy link
Author

biogerm commented Sep 5, 2016

Hi Community,

We work in Ericsson and has been using and improving Openstack4j for 2 years. Only until recently did we get the permit to contribute our changes back to Github, and we are so happy about it.

I am currently working on modifying and upstreaming these changes here. These changes are in general working fine with us for a long time. I will group them in several pull requests that I hope they won't dependent on each other.

BR
Qin An

@vinodborole
Copy link
Contributor

@biogerm we would really like to have those changes; please do send those PR and we can look at it.

Thanks again for your contribution!

@gondor
Copy link
Member

gondor commented Sep 7, 2016

@biogerm Awesome! looking forward to the contributions

This change is contributed by Elina Meier, Qin An(@biogerm)
It's modified and adapted to openstack4j 3.x by Qin An(@biogerm)
@biogerm
Copy link
Author

biogerm commented Sep 8, 2016

@vinodborole Test is now included. Just wondering if I should send it as another pull request or it's fine to have two commits?
@gondor Thanks

@biogerm
Copy link
Author

biogerm commented Sep 8, 2016

things are breaking.. i'm on it..

This change is contributed by Elina Meier, Qin An(@biogerm)
It's modified and adapted to openstack4j 3.x by Qin An(@biogerm)
@auhlig
Copy link
Member

auhlig commented Sep 8, 2016

LGTM

@gondor
Copy link
Member

gondor commented Sep 12, 2016

@vinodborole awaiting your final review. Merge if it looks good

@vinodborole
Copy link
Contributor

apologies for the delay, LGTM

@vinodborole vinodborole merged commit a61f689 into ContainX:master Sep 13, 2016
@gondor gondor added this to the 3.0.3 Release milestone Sep 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants