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

Conversation

sillydong
Copy link

This PR is adding IAM apis into the project:

**We have fixed the fixture and all pass. The acceptance still have few fail. According to ZhangDong, we keep it remaind. **:

@coveralls
Copy link

coveralls commented Apr 3, 2018

Pull Request Test Coverage Report for Build 99

  • 123 of 137 (89.78%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 85.272%

Changes Missing Coverage Covered Lines Changed/Added Lines %
openstack/identity/v3/users/requests.go 41 44 93.18%
openstack/identity/v3/roles/requests.go 54 65 83.08%
Totals Coverage Status
Change from base Build 97: 0.3%
Covered Lines: 1708
Relevant Lines: 2003

💛 - Coveralls

Copy link
Contributor

@freesky-edward freesky-edward left a comment

Choose a reason for hiding this comment

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

looks good to me, only two optional comments from me.


// Extra is a collection of miscellaneous key/values.
Extra map[string]interface{} `json:"-"`
Extra map[string]interface{} `json:"-,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

if the field tag is "-", the field is always omitted, I am not sure the purpose of this change. could you please check whether is it a maloperation.

// object storage package.
func NewObjectStorageV1(client *golangsdk.ProviderClient, eo golangsdk.EndpointOpts) (*golangsdk.ServiceClient, error) {
return initClientOpts(client, eo, "object-store")
return initClientOpts(client, eo, "object")
Copy link
Contributor

Choose a reason for hiding this comment

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

object-store is quite different from object. one is a service of swift service, another is OBS service. does this propose to remove swift support? if so, I suggest to change one other method name.

Copy link
Author

Choose a reason for hiding this comment

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

Use "object-store" will cause "No suitable endpoint could be found in the service catalog." in acceptance tests. I don't know if the resources is currently not published?

Copy link
Contributor

@freesky-edward freesky-edward Apr 9, 2018

Choose a reason for hiding this comment

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

that may cause by the service unavailable. I recommend to leave this method as it was, comment it out or remove it util the service is available. in the meantime, it would be much better to add a new method when you are going to support object service.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I will make another commit

@ShiChangkuo
Copy link
Collaborator

@sillydong Hi, Thanks for your effort.
Since our team has updated the APIs of identity v3, I will close this PR. If you have any questions, please reopen it.

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.

4 participants