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

Adopt service catalog API changes #2155

Merged
merged 1 commit into from
Oct 9, 2017
Merged

Adopt service catalog API changes #2155

merged 1 commit into from
Oct 9, 2017

Conversation

spadgett
Copy link
Member

@spadgett spadgett commented Sep 25, 2017 •
edited
Loading

https://trello.com/c/y1lbFrbp

Bumps origin-web-common 0.0.63 -> 0.0.64
Bumps origin-web-catalog 0.0.52 -> 0.0.53

Requires openshift/origin-web-catalog#448
Requires openshift/origin-web-common#199

@jeff-phillips-18 FYI

@spadgett spadgett force-pushed the plans-resource branch 2 times, most recently from 29d637e to 758ea84 Compare September 25, 2017 18:45
@spadgett spadgett mentioned this pull request Sep 25, 2017
Merged
@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 27, 2017
@spadgett spadgett force-pushed the plans-resource branch 3 times, most recently from 69a1fb4 to 2d771ff Compare September 27, 2017 20:14
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 27, 2017
@spadgett spadgett force-pushed the plans-resource branch 7 times, most recently from 990e878 to 2b6ab49 Compare October 3, 2017 14:33
@spadgett
Copy link
Member Author

spadgett commented Oct 3, 2017

@jeff-phillips-18 PTAL

@jeff-phillips-18
Copy link
Member

I think we need to go thru the rest of the code and replace creators of the resource objects that are not using the APIService.getPreferredVersion to get the resource. Places like:
https://github.com/spadgett/origin-web-console/blob/plans-resource/app/scripts/directives/resourceServiceBindings.js#L86-L89

will break when we get the service catalog changes.

@spadgett
Copy link
Member Author

spadgett commented Oct 4, 2017

Yeah it's a good idea. I can take a swing at it.

@spadgett spadgett force-pushed the plans-resource branch 2 times, most recently from de3b4ea to 76e165d Compare October 4, 2017 15:55
@spadgett
Copy link
Member Author

spadgett commented Oct 4, 2017

@jeff-phillips-18 Thanks, updated.

I'm considering making some changes to not always pull all service plans in these PRs, but that could also be a follow on. It's potentially a lot of data.

@jeff-phillips-18
Copy link
Member

Also, there is a Catalog service (in origin-web-catalog) that retrieves the service plans so a lot of this could be replaced with Catalog.getServicePlans. Maybe add some other utilities to getServicePlansForServiceClass and getServicePlanByName.

@spadgett spadgett force-pushed the plans-resource branch 5 times, most recently from be583fa to c205d6e Compare October 5, 2017 18:04
@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 5, 2017
@spadgett
Copy link
Member Author

spadgett commented Oct 5, 2017

Rebased and updated for #2215

@spadgett spadgett mentioned this pull request Oct 5, 2017
Merged
@spadgett
Copy link
Member Author

spadgett commented Oct 6, 2017

Squashed and rebased

@spadgett
Copy link
Member Author

spadgett commented Oct 6, 2017

Origin changes: openshift/origin#16689

@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 6, 2017
@spadgett
Copy link
Member Author

spadgett commented Oct 6, 2017

[test]

@spadgett spadgett changed the title [DO NOT MERGE] [WIP] Adopt service catalog API changes Adopt service catalog API changes Oct 9, 2017
@spadgett
Copy link
Member Author

spadgett commented Oct 9, 2017

@jeff-phillips-18 OK to merge? The upstream changes have merged.

@spadgett
Copy link
Member Author

spadgett commented Oct 9, 2017

Thanks @jeff-phillips-18

[merge]

@spadgett
Adopt service catalog API changes
a04fb60 
Bumps origin-web-common 0.0.63 -> 0.0.64
Bumps origin-web-catalog 0.0.52 -> 0.0.53
@spadgett
Copy link
Member Author

spadgett commented Oct 9, 2017

Rebased

@openshift-bot
Copy link

Evaluated for origin web console test up to a04fb60

@openshift-bot
Copy link

Evaluated for origin web console merge up to a04fb60

@openshift-bot
Copy link

openshift-bot commented Oct 9, 2017 •
edited
Loading

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin_web_console/324/) (Base Commit: 436cb11) (PR Branch Commit: a04fb60)

@openshift-bot
Copy link

Origin Web Console Test Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_web_console/307/) (Base Commit: 436cb11) (PR Branch Commit: a04fb60)

@openshift-bot openshift-bot merged commit ea7477e into openshift:master Oct 9, 2017
@spadgett spadgett deleted the plans-resource branch October 9, 2017 12:38
@spadgett spadgett mentioned this pull request Oct 9, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeff-phillips-18 jeff-phillips-18 jeff-phillips-18 approved these changes

Assignees
No one assigned
Labels
size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@spadgett @jeff-phillips-18 @openshift-bot @openshift-ci-robot