refactor: Migrate away from grpc_utils namespace

[mr-salty]

Fixes #1261

---

This change is 

[devjgm]

FYI, these unqualified calls to MakeStatusFromRpcError() will use ADL. That may be OK. but qualifying them with google::cloud:: would avoid the ADL and make their location clear. It was previously not using ADL. Not a huge deal either way.

I'll let you decide if you'd like to change anything.

[codecov]

Codecov Report

Merging #1266 into master will decrease coverage by <.01%.
The diff coverage is 22.5%.

@@ Coverage Diff @@ ## master #1266 +/- ## ========================================== - Coverage 94.85% 94.85% -0.01%  ========================================== Files 181 181 Lines 13794 13793 -1 ========================================== - Hits 13084 13083 -1  Misses 710 710

Impacted Files	Coverage Δ
google/cloud/spanner/internal/session_pool.cc	96.37% <ø> (ø)	⬆️
google/cloud/spanner/connection_options.cc	97.29% <ø> (ø)	⬆️
google/cloud/spanner/database_admin_client.cc	100% <ø> (ø)	⬆️
google/cloud/spanner/connection_options.h	100% <ø> (ø)	⬆️
...ogle/cloud/spanner/internal/database_admin_stub.cc	83.11% <0%> (ø)	⬆️
...ogle/cloud/spanner/internal/instance_admin_stub.cc	83.13% <0%> (ø)	⬆️
.../spanner/benchmarks/multiple_rows_cpu_benchmark.cc	92.37% <100%> (ø)	⬆️
google/cloud/spanner/connection_options_test.cc	100% <100%> (ø)	⬆️
google/cloud/spanner/internal/polling_loop.h	94.59% <100%> (+2.7%)	⬆️
google/cloud/spanner/internal/connection_impl.cc	96.25% <100%> (ø)	⬆️
... and 3 more

---

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 d250c48...6dc2428. Read the comment docs.

[mr-salty]

---

google/cloud/spanner/internal/database_admin_stub.cc, line 49 at r1 (raw file):

Previously, devjgm (Greg Miller) wrote…

FYI, these unqualified calls to MakeStatusFromRpcError() will use ADL. That may be OK. but qualifying them with google::cloud:: would avoid the ADL and make their location clear. It was previously not using ADL. Not a huge deal either way.

I'll let you decide if you'd like to change anything.

Blah. I'm happy to go either way as long as we're consistent - LMK what you think.

Some calls (in other files) were already using grpc_utils::MakeStatusFromRpcError() which turned into an unqualified call in this CL, so I decided to make them all look the same - but I didn't consider ADL, I was only thinking about the enclosing namespace ::google::cloud.

[devjgm]

Reviewable status: 0 of 13 files reviewed, all discussions resolved (waiting on @coryan and @devbww)

---

google/cloud/spanner/internal/database_admin_stub.cc, line 49 at r1 (raw file):

Previously, mr-salty (Todd Derr) wrote…

Blah. I'm happy to go either way as long as we're consistent - LMK what you think.

Some calls (in other files) were already using grpc_utils::MakeStatusFromRpcError() which turned into an unqualified call in this CL, so I decided to make them all look the same - but I didn't consider ADL, I was only thinking about the enclosing namespace ::google::cloud.

I'm happy with whatever you decide. I don't know if we have or need a hard rule for this. In general, I think the heuristic that I tend to use is that an unqualified function call is likely defined in the same file or at least the same namespace as what I'm calling. But if the function is in some other namespace, I'd expect some namespace qualification. I would probably perl -p -i -e 's,MakeStatusFromRpcError,google::cloud::MakeStatusFromRpcError,g' **.{h,cc} in this case. But if that makes things too inconsistent with other things we're doing, then I'm happy to go w/ consistency until we decide otherwise.

Up to you. Whatever LGTM.

[mr-salty]

---

google/cloud/spanner/internal/database_admin_stub.cc, line 49 at r1 (raw file):

Previously, devjgm (Greg Miller) wrote…

I'm happy with whatever you decide. I don't know if we have or need a hard rule for this. In general, I think the heuristic that I tend to use is that an unqualified function call is likely defined in the same file or at least the same namespace as what I'm calling. But if the function is in some other namespace, I'd expect some namespace qualification. I would probably perl -p -i -e 's,MakeStatusFromRpcError,google::cloud::MakeStatusFromRpcError,g' **.{h,cc} in this case. But if that makes things too inconsistent with other things we're doing, then I'm happy to go w/ consistency until we decide otherwise.

Up to you. Whatever LGTM.

Having been bitten before by ADL with make_unique<>, I'll change it.

I think we should have a guideline, just for the sake of consisistency - in the old code we were mixing the two, even within the same file. Of course without a clang-tidy it's going to be easy to do that again. I don't see a clang-tidy about ADL, I wonder if such a thing would be feasible to do and useful, or if it would have too many false positives - it would probably have to be disabled for operators at least.