Skip to content

Conversation

@plamut
Copy link

@plamut plamut commented Oct 19, 2019

Fixes #9499.
Supersedes #9500.

Following the comment on the original PR, I am submitting the changes directly to a related PR to avoid merge conflicts when merging all the work into the main Google Cloud Python repository.

@plamut
Copy link
Author

plamut commented Oct 19, 2019

@emar-kar I left the location argument in a few places in the samples where I thought we want to keep it to demonstrate its use cases (e.g. explicitly setting the location of a new dataset).

The parameter can sometimes confuse new BigQuery developers. Since
location autodetection now works pretty well, the parameter can be
removed from code samples for better clarity, except where the samples
want to explicitly demonstrate its usage.
@plamut
Copy link
Author

plamut commented Oct 21, 2019

@emar-kar Any chance of merging this? This way we avoid potential merge conflicts when you guys make any changes to the new-samples branch.

Thanks! :)

@emar-kar
Copy link

@plamut I think you need to ask Tim Swast for the review first. He should confirm that the location parameter should be deleted not only at the query methods. And I assume he will ask to merge this PR directly into google-cloud-python repo, instead of double merge through another PR.

@plamut
Copy link
Author

plamut commented Oct 21, 2019

@emar-kar Since my set of changes is more trivial, I'm fine with waiting until the new-samples branch is merged in the main repo. Will be easier to resolve any further merge conflicts on my side.

@tswast
Copy link

tswast commented Oct 25, 2019

I'm fine reviewing this PR here now that I found it in my notifications. :-)

@emar-kar emar-kar merged commit 04555f9 into MaxxleLLC:new-samples Oct 25, 2019
@plamut plamut deleted the iss-9499-b branch October 25, 2019 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants