Adding missing "hosts" field to OpenStack Nova AvailabilityZone#806
Adding missing "hosts" field to OpenStack Nova AvailabilityZone#806arvindn05 wants to merge 1 commit intojclouds:1.9.xfrom arvindn05:1.9.x
Conversation
There was a problem hiding this comment.
Make a new test and keep the original test until the deprecated code is removed.
There was a problem hiding this comment.
Same for the other tests
|
i am creating tests named DeprecatedAvailabilityZoneApiExpectTest and DeprecatedAvailabilityZonesApiLiveTest to test the deprecated code. If you guys have a better naming pattern, i can use that. |
|
@zack-shoylev I added the the tests for deprecated code as well. |
|
Ok. Thanks, ran it and looks good, but I have some questions: It seems you need to deprecate, because you are using a new AvailabilityZone class. Why not just update the old availability zone so you don't have to deprecate it? It seems the new one is just adding stuff, so it should be backwards compatible. Also, while the rest of nova might not use AutoValue yet, maybe it makes sense to start using it here. Let me know if I missed something! |
|
everything under the zone scoped package is being deprecated. If you look at the original bug request, it has the bullet point below Move AvailabilityZone and ZoneState from the deprecated org.jclouds.openstack.nova.v2_0.domain.zonescoped package to the correct org.jclouds.openstack.nova.v2_0.domain package. I am not too familiar with autovalue. Since this is just adding stuff to fix a bug i would prefer to keep using the existing mechanism for now instead of moving to auto value. |
|
Thanks for explaining! In this case, this should be good to merge - rebase to a single commit. |
|
@zack-shoylev did i get the rebase to single commit right this time? Or else i can create a new pull request with just the single commit. Also, the bugs which i fixed are tracked as part of JIRA. Do i need to do anything to update their status? |
|
Hm, no, that seems to have messed it up again. What I do: In my PR branch, I rebase the last 2 (or more) commits: git rebase -i HEAD~2 Then I force push to the github branch I used for the pull request: git push -f When you go to the PR page on github, you should only see 1 commit in the commits tab. |
|
@zack-shoylev Thanks for the tip. It looks like its fixed now. I see only one commit in PR page. |
|
merged, backported. Thanks! |
2 participants
Adding missing "hosts" field to OpenStack Nova AvailabilityZone.
Fixes bug https://issues.apache.org/jira/browse/JCLOUDS-700