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

Release Proposal: v2.13 #318

Merged
merged 1 commit into from
Sep 27, 2017
Merged

Release Proposal: v2.13 #318

merged 1 commit into from
Sep 27, 2017

Conversation

mattmcneeney
Copy link
Contributor

@mattmcneeney mattmcneeney commented Sep 19, 2017 •
edited by avade
Loading

This triggers the start of the Review Process

edit by @avade

Review is open until Tuesday 26th September

@cfdreddbot
Copy link

Hey mattmcneeney!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@mattmcneeney mattmcneeney self-assigned this Sep 19, 2017
@angarg12
Copy link
Contributor

Travis is complaining about links, but I guess it is unavoidable since those URLs won't exists until the PR is merged, is that right?

avade
avade reviewed Sep 19, 2017
View reviewed changes
release-notes.md
## <a id='2-12'></a>v2.12 ##
# Service Broker API Release Notes

## v2.13
Copy link
Contributor

Choose a reason for hiding this comment

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

I know we don't have this for previous releases, but links from the feature to the relevant part of the spec would be very helpful. I can do this post merge.

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

avade
avade reviewed Sep 19, 2017
View reviewed changes
spec.md
* Clarified the semantics of `plan_id` and `parameters` fields in the updating a service instance request body.
* Clarified the default value of the `plan_updateable` field.
* Clarified when `route_service_url` is REQUIRED and when brokers can return extra data in bindings.
### Changes Since v2.12
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, links here to the feature would be very helpful.

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

@avade
Copy link
Contributor

avade commented Sep 19, 2017

Looks like you missed updating the ToC to changes since v2.12 https://github.com/mattmcneeney/servicebroker/blob/c6cd947f6384a90db26e072004bca6413e0bf79f/spec.md#table-of-contents

@mattmcneeney
Copy link
Contributor Author

Thanks @avade - fixing now

@mattmcneeney
Copy link
Contributor Author

@angarg12 Yep links won't work until the v2.13 branch is created (I can do this now if people would prefer so we can test this?). I've tested the links replacing v2.13 with master and they all work.

@angarg12
Copy link
Contributor

@mattmcneeney that's ok as long as we are aware of the fact :). However in the future it may be distracting that the build fails, making it harder to detect genuine failures. We can discuss it if it is relevant for other people.

@duglin
Copy link
Contributor

duglin commented Sep 19, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

@avade
Copy link
Contributor

avade commented Sep 19, 2017 •
edited by chipchilders
Loading

LGTM now thanks!

Approved with PullApprove

@zrob
Copy link
Member

zrob commented Sep 19, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

@mattmcneeney
Copy link
Contributor Author

mattmcneeney commented Sep 19, 2017 •
edited by chipchilders
Loading

One more LGTM needed, but do not merge until after Tuesday 26th September as per the review process.

Approved with PullApprove

@angarg12
Copy link
Contributor

angarg12 commented Sep 19, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

@Samze Samze mentioned this pull request Sep 20, 2017
Merged
4 tasks
@vaikas
Copy link
Contributor

vaikas commented Sep 21, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

@jmrodri
Copy link

jmrodri commented Sep 21, 2017

Great, this is exciting. LGTM

@mattmcneeney
Copy link
Contributor Author

mattmcneeney commented Sep 26, 2017 •
edited by chipchilders
Loading

I squeezed in #222 so this needs another round of LGTMs...

Approved with PullApprove

@vaikas
Copy link
Contributor

vaikas commented Sep 26, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

@avade
Copy link
Contributor

avade commented Sep 26, 2017 •
edited by chipchilders
Loading

lgtm - I think it's important that was included in this release @mattmcneeney - helps out the bluemix folks. Nice one.

Approved with PullApprove

@duglin
Copy link
Contributor

duglin commented Sep 26, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

1 similar comment
@shalako
Copy link
Contributor

shalako commented Sep 26, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

@mattmcneeney mattmcneeney merged commit 15f408f into openservicebrokerapi:master Sep 27, 2017
@mattmcneeney
Copy link
Contributor Author

Creating the release now! 🎉

@mattmcneeney mattmcneeney deleted the v2.13-rc branch January 19, 2018 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@avade avade avade left review comments

Assignees

@mattmcneeney mattmcneeney

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@mattmcneeney @cfdreddbot @angarg12 @avade @duglin @zrob @vaikas @jmrodri @shalako