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 the zone parameter to create_subnet #101

Merged
merged 11 commits into from Aug 2, 2018
Merged

Conversation

Dyex719
Copy link
Contributor

@Dyex719 Dyex719 commented Oct 27, 2017

Adding the zone parameter to create_subnet

Adding the zone parameter to create_subnet
Copy link
Contributor

@nuwang nuwang left a comment

Choose a reason for hiding this comment

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

Thanks for continuing your work on this. I've added a comment. The tests are also still failing: https://travis-ci.org/gvlproject/cloudbridge

@@ -740,7 +740,7 @@ def find(self, name, limit=None, marker=None):
pass

@abstractmethod
def create(self, name, network_id, cidr_block, zone=None):
def create(self, name, network_id, cidr_block, zone=zone):
Copy link
Contributor

Choose a reason for hiding this comment

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

This should just be a required parameter, with no default value, as follows:

def create(self, name, network_id, cidr_block, zone):

@afgane
Copy link
Contributor

afgane commented Oct 30, 2017

If you rebase your changes on top of this commit 18e3c04, the tests should pass.

@Dyex719
Copy link
Contributor Author

Dyex719 commented Oct 30, 2017

Does this mean that I have to create a new pull request on top of the new commit that you made? I haven't done rebasing before...

@afgane
Copy link
Contributor

afgane commented Oct 31, 2017

You can just merge the master branch into your fork. I think the following should take care of it:

git remote add upstream https://github.com/gvlproject/cloudbridge
git fetch upstream
git checkout dev
git merge upstream/master

That should create a merge commit. Ideally, you could then rebase this for a cleaner history but unless you're eager to learn the process, don't worry about it (and if you do decide to learn, push your changes first because with rebase there's a chance of messing up your local cone).

@coveralls
Copy link

coveralls commented Oct 31, 2017

Coverage Status

Coverage remained the same at 70.129% when pulling 88cb142 on Dyex719:master into 4f5b4ba on gvlproject:master.

@nuwang
Copy link
Contributor

nuwang commented Dec 1, 2017

@Dyex719 Are you planning to do more work on this (see here: #63 (comment)) or should we merge in the changes you've made so far?

@Dyex719
Copy link
Contributor Author

Dyex719 commented Dec 1, 2017

Sorry, I'm busy till 15th. I can only work on this after that. Is that okay?

@nuwang
Copy link
Contributor

nuwang commented Dec 1, 2017

No worries, was just following up on this.

@codecov-io
Copy link

Codecov Report

Merging #101 into master will increase coverage by 29.81%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #101       +/-   ##
===========================================
+ Coverage   38.66%   68.48%   +29.81%     
===========================================
  Files          23       17        -6     
  Lines        4911     3167     -1744     
  Branches      380      299       -81     
===========================================
+ Hits         1899     2169      +270     
+ Misses       3005      946     -2059     
- Partials        7       52       +45
Impacted Files Coverage Δ
cloudbridge/cloud/providers/aws/services.py 88.23% <100%> (+46.74%) ⬆️
cloudbridge/cloud/providers/openstack/services.py 31.58% <100%> (+2.26%) ⬆️
cloudbridge/cloud/factory.py 100% <0%> (ø) ⬆️
cloudbridge/cloud/base/helpers.py
cloudbridge/cloud/providers/azure/__init__.py
cloudbridge/cloud/providers/azure/azure_client.py
cloudbridge/cloud/providers/azure/helpers.py
cloudbridge/cloud/providers/azure/resources.py
cloudbridge/cloud/providers/azure/provider.py
cloudbridge/cloud/providers/azure/services.py
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6bc54d2...6883bca. Read the comment docs.

@afgane afgane changed the title Issue #63 Add the zone parameter to create_subnet Jul 14, 2018
@afgane afgane merged commit 6883bca into CloudVE:master Aug 2, 2018
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

5 participants