Skip to content
This repository was archived by the owner on Aug 31, 2021. It is now read-only.

Conversation

edisonxiang
Copy link
Contributor

What this PR does / why we need it: add replication resource

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release note:

NONE

@edisonxiang edisonxiang changed the title add replication resource add drs service Mar 6, 2018
@edisonxiang edisonxiang requested a review from zengchen1024 March 6, 2018 02:49
@coveralls
Copy link

coveralls commented Mar 6, 2018

Pull Request Test Coverage Report for Build 13

  • 0 of 4 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.06%) to 62.503%

Changes Missing Coverage Covered Lines Changed/Added Lines %
openstack/client.go 0 4 0.0%
Totals Coverage Status
Change from base Build 5: -0.06%
Covered Lines: 2412
Relevant Lines: 3859

💛 - Coveralls

type CreateOps struct {
// The name of the EVS replication pair.
// The name can contain a maximum of 255 bytes.
Name string `q:"name"`
Copy link
Contributor

Choose a reason for hiding this comment

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

If you don't add any more tags, like required or omitempty, this field will be send to backend. If this filed can not be empty string, please add the tag of 'required="true"'

)

// RequestOpts specifies the header
var RequestOpts = golangsdk.RequestOpts{
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, you do not need this variable. Take the 'Content-Type' for example. It will be add at https://github.com/huaweicloud/golangsdk/blob/master/provider_client.go#L193
Please add more comments to make it clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}

_, r.Err = client.Post(createURL(client), b, &r.Body, &golangsdk.RequestOpts{
OkCodes: []int{202, 201, 200},
Copy link
Contributor

Choose a reason for hiding this comment

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

According to my experience, the return code should be only one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


// Delete a replication by id
func Delete(client *golangsdk.ServiceClient, id string) (r DeleteResult) {
_, r.Err = client.Delete(deleteURL(client, id), &RequestOpts)
Copy link
Contributor

Choose a reason for hiding this comment

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

No need specify 'RequestOpts'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


// Get a replication with detailed information by id
func Get(client *golangsdk.ServiceClient, id string) (r GetResult) {
_, r.Err = client.Get(getURL(client, id), &r.Body, &RequestOpts)
Copy link
Contributor

Choose a reason for hiding this comment

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

No need specify 'RequestOpts'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@zengchen1024
Copy link
Contributor

by the way, you can use the tool to generate these go files.

@edisonxiang
Copy link
Contributor Author

@zengchen1024 Thanks very much:)

@edisonxiang edisonxiang merged commit 463960d into huaweicloud:master Mar 7, 2018
@edisonxiang edisonxiang deleted the adddrsservice branch March 7, 2018 02:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants