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

doc: consolidate and update README.md, INSTALL.md#1026

Merged
scotthart merged 4 commits intogoogleapis:masterfrom
scotthart:release_201911
Nov 6, 2019
Merged

doc: consolidate and update README.md, INSTALL.md#1026
scotthart merged 4 commits intogoogleapis:masterfrom
scotthart:release_201911

Conversation

@scotthart
Copy link
Copy Markdown
Member

@scotthart scotthart commented Nov 6, 2019 •
edited by coryan
Loading

This change consolidates the release notes into the top level README.md and removes google/cloud/spanner/README.md

This change is Reviewable

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 6, 2019
@scotthart
chore: update README
b9369cc 
remove subdir README.md

chore: remove subdir README.md
coryan
coryan suggested changes Nov 6, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@coryan coryan left a comment

Choose a reason for hiding this comment

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

I would say that this is a doc: improvement more than a chore:, but I do not know for sure.

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @devjgm, @mr-salty, and @scotthart)

README.md, line 3 at r1 (raw file):

# Google Cloud Spanner C++ Client Library

<!-- This file is automatically generated by ci/test-readme/generate-readme.sh -->"

Note that this file is automatically generated. I think we want to change the generator?

scotthart
scotthart commented Nov 6, 2019
View reviewed changes
Copy link
Copy Markdown
Member Author

@scotthart scotthart left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 5 files reviewed, 1 unresolved discussion (waiting on @coryan, @devjgm, and @mr-salty)

README.md, line 3 at r1 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

Note that this file is automatically generated. I think we want to change the generator?

Done.

@scotthart scotthart changed the title chore: consolidate README.md doc: consolidate and update README.md, INSTALL.md Nov 6, 2019
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 6, 2019 •
edited
Loading

Codecov Report

Merging #1026 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1026      +/-   ##
==========================================
- Coverage    93.7%   93.69%   -0.02%     
==========================================
  Files         158      158              
  Lines       11187    11185       -2     
==========================================
- Hits        10483    10480       -3     
- Misses        704      705       +1
Impacted Files Coverage Δ
google/cloud/spanner/internal/retry_loop.h 95.45% <0%> (-4.55%) ⬇️
google/cloud/spanner/internal/polling_loop.h 91.89% <0%> (-2.71%) ⬇️
google/cloud/spanner/internal/spanner_stub.cc 66.66% <0%> (-2.23%) ⬇️
...loud/spanner/internal/partial_result_set_source.cc 92.95% <0%> (-1.41%) ⬇️
...ud/spanner/integration_tests/client_stress_test.cc 82.45% <0%> (-0.88%) ⬇️
...on_tests/rpc_failure_threshold_integration_test.cc 85.55% <0%> (-0.16%) ⬇️
...anner/integration_tests/client_integration_test.cc 99.45% <0%> (-0.01%) ⬇️
google/cloud/spanner/samples/samples.cc 90.16% <0%> (+0.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99bf734...f109430. Read the comment docs.

coryan
coryan suggested changes Nov 6, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@coryan coryan left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r2.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @devjgm, @mr-salty, and @scotthart)

README.md, line 123 at r1 (raw file):

Apache 2.0; see [`LICENSE`](LICENSE) for details.

Hmm... seems like you found a bug in the generator? Maybe we should preserve that blank line?

README.md, line 52 at r2 (raw file):

**Install Instructions**
[![CI status install/centos-7][install/centos-7-shield]][install/centos-7-link]

I think you discovered a bug in the generator for this file. I believe all these badges are not created yet, but the scripts assume they are. We can release with the broken badges (not great) if we can schedule the builds ~now. Or we can manually edit the file this one time and release with the new badges later.

I think the latter is the better option with so many of us out, what do you think"?

README.md, line 93 at r2 (raw file):

[docker/ubsan-link]: https://storage.googleapis.com/cloud-cpp-kokoro-status/spanner/docker/ubsan-link.html
[docker/ubsan-shield]: https://storage.googleapis.com/cloud-cpp-kokoro-status/spanner/docker/ubsan.svg
[install/centos-7-link]: https://storage.googleapis.com/cloud-cpp-kokoro-status/spanner/install/centos-7-link.html

Same for these badges.

doc/cutting-a-release.md, line 49 at r2 (raw file):

```bash

Thanks!

scotthart
scotthart commented Nov 6, 2019
View reviewed changes
Copy link
Copy Markdown
Member Author

@scotthart scotthart left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @coryan, @devjgm, and @mr-salty)

README.md, line 123 at r1 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

Hmm... seems like you found a bug in the generator? Maybe we should preserve that blank line?

Done.

README.md, line 52 at r2 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

I think you discovered a bug in the generator for this file. I believe all these badges are not created yet, but the scripts assume they are. We can release with the broken badges (not great) if we can schedule the builds ~now. Or we can manually edit the file this one time and release with the new badges later.

I think the latter is the better option with so many of us out, what do you think"?

Done.

README.md, line 93 at r2 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

Same for these badges.

Done.

doc/cutting-a-release.md, line 49 at r2 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

Thanks!

Done.

coryan
coryan approved these changes Nov 6, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@coryan coryan left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 2 files at r3.
Reviewable status: 5 of 6 files reviewed, all discussions resolved (waiting on @devjgm and @mr-salty)

@scotthart scotthart merged commit 661640f into googleapis:master Nov 6, 2019
devjgm pushed a commit to devjgm/google-cloud-cpp that referenced this pull request May 7, 2020
@scotthart
doc: consolidate and update README.md, INSTALL.md (googleapis/google-…
6011e90 
…cloud-cpp-spanner#1026)

* chore: remove subdir README.md
* doc: remove broken install links for this release
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mr-salty mr-salty Awaiting requested review from mr-salty

@devjgm devjgm Awaiting requested review from devjgm

1 more reviewer

@coryan coryan coryan 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.

3 participants

@scotthart @coryan @googlebot