Skip to content

Release Spanner v0.30.0 #4732

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
Merged
theacodes merged 1 commit into from
Jan 11, 2018
Merged

Release Spanner v0.30.0 #4732

theacodes merged 1 commit into from
Jan 11, 2018

Conversation

@theacodes
Copy link
Contributor

@theacodes theacodes commented Jan 10, 2018

No description provided.

@theacodes theacodes added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jan 10, 2018
@theacodes theacodes requested a review from tseaver as a code owner January 10, 2018 22:57
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 10, 2018
@theacodes
Copy link
Contributor Author

theacodes commented Jan 10, 2018

Must be rebased on #4730 before merging.

dhermes
dhermes approved these changes Jan 10, 2018
View reviewed changes
Copy link
Contributor

@dhermes dhermes left a comment

Choose a reason for hiding this comment

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

LGTM mostly

spanner/CHANGELOG.md Outdated

### Implementation Changes

- The underlying autogenerated client library was re-generated to pick up new features and resolve bugs. (#4695)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

spanner/CHANGELOG.md Outdated

- The underlying autogenerated client library was re-generated to pick up new features and resolve bugs. (#4695)
- Made sure **exactly** one of `start_*`/`end_*` are passed to KeyRange. (#4618)
- Made `row`, `consume_all`, and `consume_next` private (#4492)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

spanner/CHANGELOG.md

- Brought Spanner README more in line with others. (#4306, #4317)

### Testing

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tseaver
Copy link
Contributor

tseaver commented Jan 10, 2018

I still believe that the validation change in the KeyRange constructor in #4618 was ill-advised (see #4694). If we leave it in place for this release, it should be highlighted as a breaking change.

@dhermes
Copy link
Contributor

dhermes commented Jan 10, 2018

@tseaver I was going to mention that in my review, thanks for pointing it out.

@tseaver
Copy link
Contributor

tseaver commented Jan 10, 2018

#4492 is a breaking change, as well: it removed the rows property and the consume_all method, making consume_next private.

@theacodes
Copy link
Contributor Author

theacodes commented Jan 10, 2018

Reverting it. We can revisit it at a later time if needed.

@theacodes theacodes force-pushed the release-spanner-v0.30.0 branch from 680c451 to 4efea87 Compare January 11, 2018 00:57
@theacodes
Copy link
Contributor Author

theacodes commented Jan 11, 2018

Rebased and good to merge when CI is green and api_core 0.1.4 is released (#4730)

@theacodes theacodes mentioned this pull request Jan 11, 2018
Merged
@chemelnucfin
Copy link
Contributor

chemelnucfin commented Jan 11, 2018

@jonparrott That change was suggested by @vkedia

@theacodes
Copy link
Contributor Author

theacodes commented Jan 11, 2018 •
edited
Loading

@chemelnucfin two different breaking changes were discussed, which were you referring to? The change of making a few members private was not reverted and I'm totally cool with it.

@chemelnucfin
Copy link
Contributor

chemelnucfin commented Jan 11, 2018 via email

@theacodes theacodes force-pushed the release-spanner-v0.30.0 branch from 4efea87 to ded35a1 Compare January 11, 2018 17:26
@theacodes
Copy link
Contributor Author

theacodes commented Jan 11, 2018

#4735 is now included, thanks, @chemelnucfin.

Waiting for CI and then merging. :)

@theacodes theacodes merged commit 55fd0ff into master Jan 11, 2018
@theacodes theacodes mentioned this pull request Jan 11, 2018
Closed
9 tasks
@theacodes theacodes removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jan 11, 2018
@theacodes theacodes deleted the release-spanner-v0.30.0 branch February 14, 2018 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lukesneeringer lukesneeringer Awaiting requested review from lukesneeringer

@tseaver tseaver Awaiting requested review from tseaver

1 more reviewer

@dhermes dhermes dhermes approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@theacodes @tseaver @dhermes @chemelnucfin @googlebot