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.

Sign up for GitHub

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

Jump to bottom

resource to API 500 Synergy / API 300 Synergy #371

Merged
merged 5 commits into from
Apr 4, 2019
Merged

resource to API 500 Synergy / API 300 Synergy #371

merged 5 commits into from
Apr 4, 2019

Conversation

riconem
Copy link
Contributor

@riconem riconem commented Mar 19, 2019 •
edited
Loading

Description

The resource was set to C7000, but most of the Logical Interconnect Group functionality for the OneView are build in '../../api300/synergy'.
The recource for api500 is also set to api300.

Issues Resolved

Fixes #372

Check List

  • New functionality includes testing.
    • All tests pass ($ rake test).
  • New functionality has been documented in the README if applicable.
    • New functionality has been thoroughly documented in the examples (please include helpful comments).
  • New endpoints supported are updated in the endpoints-support.md file.
  • Changes are documented in the CHANGELOG.

@riconem
resource to API 500 Synergy
83f43ae 
The resource was set to C7000, but most of the Logical Interconnect Group functionality for the Synergy are build in '../../api300/synergy'.
The recource for api500 is also set to api300.
@riconem
Merge pull request #1 from riconem/riconem
b74d20c 
resource to API 500 Synergy
@riconem riconem changed the base branch from bugfix/integration_tests_for_create_method to master March 19, 2019 12:55
@riconem
Copy link
Contributor Author

riconem commented Mar 28, 2019

Hi @Sowmyahsg,

I saw you are an active user of the oneview-sdk-ruby module. Could you check my pullrequest?
Is there anything I missed to pull these changes? I am new in Git and puppet so maybe you could help me.

@soodpr
Copy link
Member

soodpr commented Mar 28, 2019

@riconem - Thanks for your contributions. I quickly had a look into the hierarchy issue and you are right, it needs to be inherited from Synergy, not from C7000 as main functionality is in Api300.
Can you please open an issue here so that it can be tracked later on.

@riconem
Copy link
Contributor Author

riconem commented Mar 28, 2019

@soodpr thank you for responding. 👍

soodpr
soodpr approved these changes Apr 3, 2019
View reviewed changes
Copy link
Member

@soodpr soodpr left a comment

Choose a reason for hiding this comment

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

LGTM 👍

soodpr
soodpr requested changes Apr 3, 2019
View reviewed changes
Copy link
Member

@soodpr soodpr left a comment

Choose a reason for hiding this comment

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

You need to update the unit test as well:
https://github.com/HewlettPackard/oneview-sdk-ruby/blob/master/spec/unit/resource/api600/synergy/logical_interconnect_group.rb#L6

@riconem
resource to API 500 Synergy
503c630 
The resource was set to C7000, but most of the Logical Interconnect Group functionality for the Synergy are build in '../../api300/synergy'.
The recource for api500 is also set to api300.
@soodpr
Copy link
Member

soodpr commented Apr 4, 2019

Can you please add CHANGELOG file as well and add new v5.7.1 version and add your change under Bug fixes & Enhancements heading?

@riconem
Copy link
Contributor Author

riconem commented Apr 4, 2019

Can you please add CHANGELOG file as well and add new v5.7.1 version and add your change under Bug fixes & Enhancements heading?

For sure! Should I also add the new version here?

@soodpr
Copy link
Member

soodpr commented Apr 4, 2019 •
edited
Loading

Can you please add CHANGELOG file as well and add new v5.7.1 version and add your change under Bug fixes & Enhancements heading?

For sure! Should I also add the new version here?

Ideally we should, but I need to check if we are going to release this patch now or later? So, for a time being, we can keep it like this. Lets get one more review.

soodpr
soodpr approved these changes Apr 4, 2019
View reviewed changes
Copy link
Member

@soodpr soodpr left a comment

Choose a reason for hiding this comment

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

LGTM 👍

madhav-bharadwaj
madhav-bharadwaj reviewed Apr 4, 2019
View reviewed changes
CHANGELOG.md Outdated
@@ -1,3 +1,11 @@
## v5.7.1
Copy link
Member

Choose a reason for hiding this comment

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

Since this version is unreleased, can you also add this to the version so that there is no confusion. Something like v5.7.1 (Unreleased)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All right!

madhav-bharadwaj
madhav-bharadwaj approved these changes Apr 4, 2019
View reviewed changes
Copy link
Member

@madhav-bharadwaj madhav-bharadwaj left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please feel free to reference the PR in the issue raised and close it if this fixes your issue. Thank you for your contributions. :)

@soodpr soodpr merged commit c2b1229 into HewlettPackard:master Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@madhav-bharadwaj madhav-bharadwaj madhav-bharadwaj approved these changes

@soodpr soodpr soodpr approved these changes

@AnushaY1916 AnushaY1916 Awaiting requested review from AnushaY1916

@jyothisgm jyothisgm Awaiting requested review from jyothisgm

@sijeesh sijeesh Awaiting requested review from sijeesh

@Sowmyahsg Sowmyahsg Awaiting requested review from Sowmyahsg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@riconem @soodpr @madhav-bharadwaj