Skip to content

Remove old/unused Bigtable v1 generated code#1999

Merged
dhermes merged 3 commits intogoogleapis:masterfrom
dhermes:remove-bigtable-v1
Jul 21, 2016
Merged

Remove old/unused Bigtable v1 generated code#1999
dhermes merged 3 commits intogoogleapis:masterfrom
dhermes:remove-bigtable-v1

Conversation

@dhermes
Copy link
Contributor

@dhermes dhermes commented Jul 19, 2016

@tseaver there are a lot of import aliases leftover like:

from gcloud.bigtable._generated import (
    bigtable_instance_admin_pb2 as instance_admin_v2_pb2)

Is it worth me tracking them down?

@dhermes dhermes added hygiene api: bigtable Issues related to the Bigtable API. labels Jul 19, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 19, 2016
@tseaver
Copy link
Contributor

tseaver commented Jul 19, 2016

This PR would be a lot easier to read if broken up: e.g., first remove all the _generate/*.py modules in one PR (so that we could see that there was no breakage / overlap), and then rename _generated_v2 -> _generated in a subsequent PR: at that point, maybe ditching the aliases would be worthwhile.

@dhermes
Copy link
Contributor Author

dhermes commented Jul 19, 2016

@tseaver It's broken up into 3 commits to accomplish essentially the same thing (but a little differently):

  1. Remove all v1 code
  2. Remove all v2 references in text files
  3. Rename all v2 files

@dhermes dhermes force-pushed the remove-bigtable-v1 branch from e23193c to e7f265a Compare July 21, 2016 16:55
@dhermes
Copy link
Contributor Author

dhermes commented Jul 21, 2016

@tseaver PTAL

@tseaver
Copy link
Contributor

tseaver commented Jul 21, 2016

@dhermes Your second commit couldn't possibly pass tests: the rename of the files has to go hand-in-hand with the updated imports. Could you squash it with the third commit?

@dhermes
Copy link
Contributor Author

dhermes commented Jul 21, 2016

The failure was due to a newly introduced (hence not renamed) import from #2002.

I made the commits as a group, not trying to get tests to pass in between. I avoided doing the import renames in the same commit as the file renames to make the diff easier to read. Having a "file rename only" commit makes the commit easier to grok.

@dhermes dhermes force-pushed the remove-bigtable-v1 branch from e7f265a to 5d8ec27 Compare July 21, 2016 18:28
@tseaver
Copy link
Contributor

tseaver commented Jul 21, 2016

OK, LGTM. Looking forward to the day that we can rip this junk out, and just use the grpc-google-{datastore,bigtable}-v# libraries instead.

@dhermes dhermes merged commit a316dd6 into googleapis:master Jul 21, 2016
@dhermes dhermes deleted the remove-bigtable-v1 branch July 21, 2016 19:54
This was referenced Aug 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the Bigtable API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants