cleanup!: use the google-cloud-cpp-common project #3196
Conversation
googlebot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #3196 +/- ##
==========================================
- Coverage 91.06% 90.28% -0.78%
==========================================
Files 300 448 +148
Lines 20360 39275 +18915
==========================================
+ Hits 18540 35461 +16921
- Misses 1820 3814 +1994
Continue to review full report at Codecov.
|
Needed to build google-cloud-cpp-common as shared libraries so these builds would work. Also I forgot to remove the common libraries from the check-abi.sh script.
The google-cloud-cpp-common port does not exist in vcpkg, and should not exist until this PR is merged. So for a short while we will use vcpkg overlays, once we cut the next release we can clean all of this up.
|
PTAL. |
There was a problem hiding this comment.
Reviewed 154 of 156 files at r1, 11 of 11 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @coryan)
ci/kokoro/docker/Dockerfile.fedora-install, line 95 at r2 (raw file):
RUN tar -xf v0.13.0.tar.gz WORKDIR /var/tmp/build/google-cloud-cpp-common-0.13.0 # Compile without the tests because we are testing spanner, not the base
this should say something other than "spanner".
Applies to a bunch of Dockerfiles (I only saw it go by 5 times or so before it registered...)
ci/test-install/WORKSPACE, line 51 at r2 (raw file):
) # https://github.com/bazelbuild/bazel/issues/1550
nit: get rid of extra indent since you deleted the top line
google/cloud/README.md, line 11 at r2 (raw file):
the
s/, the/; they/
|
On Mon, Oct 14, 2019 at 1:43 PM Todd Derr ***@***.***> wrote:
***@***.**** approved this pull request.
[image: <img class="emoji" title=":lgtm:" alt=":lgtm:" align="absmiddle" src="https://reviewable.io/lgtm.png" height="20" width="61"/>]
<https://camo.githubusercontent.com/41b3ec3419116e3b3f84507ad965646ce4ce1f0c/68747470733a2f2f72657669657761626c652e696f2f6c67746d2e706e67>
Reviewed 154 of 156 files at r1, 11 of 11 files at r2.
I have no idea why it says "154 of 156", I have the little green eyeball on
every file in the list... and i'm not going to count them.
… *Reviewable
<https://reviewable.io/reviews/googleapis/google-cloud-cpp/3196#-:-LrAMUEl-Mz1izunFSGN:bnfp4nl>*
status: all files reviewed, 3 unresolved discussions (waiting on @coryan
<https://github.com/coryan>)
------------------------------
*ci/kokoro/docker/Dockerfile.fedora-install, line 95 at r2
<https://reviewable.io/reviews/googleapis/google-cloud-cpp/3196#-LrAJdkSAZUXieXwmIj0:-LrAJdkT3yS_UdLE-sD5:b-ejjf1m>
(raw file
<https://github.com/googleapis/google-cloud-cpp/blob/8e9105fc92456c36cf577360b00dff4fa8340d79/ci/kokoro/docker/Dockerfile.fedora-install#L95>):*
RUN tar -xf v0.13.0.tar.gz
WORKDIR /var/tmp/build/google-cloud-cpp-common-0.13.0
# Compile without the tests because we are testing spanner, not the base
this should say something other than "spanner".
Applies to a bunch of Dockerfiles (I only saw it go by 5 times or so
before it registered...)
------------------------------
*ci/test-install/WORKSPACE, line 51 at r2
<https://reviewable.io/reviews/googleapis/google-cloud-cpp/3196#-LrAKDkO4ehr9XAmAms_:-LrAKDkP2meYVPqXUUh8:b4cm4um>
(raw file
<https://github.com/googleapis/google-cloud-cpp/blob/8e9105fc92456c36cf577360b00dff4fa8340d79/ci/test-install/WORKSPACE#L51>):*
)
# bazelbuild/bazel#1550
nit: get rid of extra indent since you deleted the top line
------------------------------
*google/cloud/README.md, line 11 at r2
<https://reviewable.io/reviews/googleapis/google-cloud-cpp/3196#-LrALMQW0vnQRGCXDzLD:-LrALMQW0vnQRGCXDzLE:b-ruwcsk>
(raw file
<https://github.com/googleapis/google-cloud-cpp/blob/8e9105fc92456c36cf577360b00dff4fa8340d79/google/cloud/README.md#L11>):*
the
s/, the/; they/
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#3196?email_source=notifications&email_token=AB2T57FB3XOLRFI6ZTQY62LQOSVVBA5CNFSM4I77DQ22YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCH34EOA#pullrequestreview-301449784>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB2T57BKXI4OLZ7UUJMFJ2LQOSVVBANCNFSM4I77DQ2Q>
.
|
There was a problem hiding this comment.
Thanks for the detailed review, I know it cannot have been easy.
Reviewable status: 152 of 165 files reviewed, 3 unresolved discussions (waiting on @mr-salty)
ci/kokoro/docker/Dockerfile.fedora-install, line 95 at r2 (raw file):
Previously, mr-salty (Todd Derr) wrote…
this should say something other than "spanner".
Applies to a bunch of Dockerfiles (I only saw it go by 5 times or so before it registered...)
Fixed here and elsewhere, thanks for noticing.
ci/test-install/WORKSPACE, line 51 at r2 (raw file):
Previously, mr-salty (Todd Derr) wrote…
nit: get rid of extra indent since you deleted the top line
Removed, I think this information is just "how Bazel works".
google/cloud/README.md, line 11 at r2 (raw file):
Previously, mr-salty (Todd Derr) wrote…
thes/, the/; they/
Fixed. Thanks!
BREAKING CHANGE: The common libraries have been moved
to the google-cloud-cpp-common repository.
While this may not be a technically breaking change (the API and ABI
remain unchanged, the include paths are the same), it will require
application developers to change their build scripts, so I think labeling
as such is useful.
This change is