Skip to content
This repository was archived by the owner on Nov 18, 2025. It is now read-only.

fix!: remove pageSize/pageToken from call settings#821

Closed
stephenplusplus wants to merge 3 commits intogoogleapis:drop_node_10from
stephenplusplus:spp--remove-page-options
Closed

fix!: remove pageSize/pageToken from call settings#821
stephenplusplus wants to merge 3 commits intogoogleapis:drop_node_10from
stephenplusplus:spp--remove-page-options

Conversation

@stephenplusplus
Copy link
Contributor

@stephenplusplus stephenplusplus commented May 12, 2020 •
edited
Loading

Marking as do not merge as this contains a breaking change.

(Otherwise, it's ready to go)

There are two API-level options that exist on the CallOptions object. This means users call (through a client library):

// spanner.getInstances(apiOptions, [gaxOptions], callback)

// Expected usage:
spanner.getInstances({ filter: '...', pageSize: 10 }, { autoPaginate: true }, (err, results) => {})

// Possible usage that is weird:                                                ↓ two pageSizes!
spanner.getInstances({ filter: '...', pageSize: 10 }, { autoPaginate: true, pageSize: 11 }, (err, results) => {})

This PR removes those options from the CallOptions object, which (hopefully) isn't deadly in terms of actual functionality.

@googlebot
Copy link

googlebot commented May 12, 2020

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label May 12, 2020
@stephenplusplus stephenplusplus force-pushed the spp--remove-page-options branch from d397dc6 to b8b56a1 Compare May 12, 2020 01:35
@googlebot
Copy link

googlebot commented May 12, 2020

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels May 12, 2020
@codecov
Copy link

codecov bot commented May 12, 2020 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (main@e93c510). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head a869743 differs from pull request most recent head 5e881dc. Consider uploading reports for the commit 5e881dc to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main     #821   +/-   ##
=======================================
  Coverage        ?   59.66%           
=======================================
  Files           ?       46           
  Lines           ?     7401           
  Branches        ?       94           
=======================================
  Hits            ?     4416           
  Misses          ?     2985           
  Partials        ?        0

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 e93c510...5e881dc. Read the comment docs.

@stephenplusplus stephenplusplus mentioned this pull request May 12, 2020
Merged
3 tasks
@alexander-fenster
Copy link
Contributor

alexander-fenster commented May 12, 2020

@stephenplusplus Are you OK if this PR will wait here in the lobby for a few months before we are ready to make the next semver major?

@stephenplusplus
Copy link
Contributor Author

stephenplusplus commented May 12, 2020

@alexander-fenster yep, no problem. Thanks!

@stephenplusplus stephenplusplus added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 12, 2020
@AVaksman AVaksman mentioned this pull request Aug 20, 2020
Closed
4 tasks
@bcoe bcoe added the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 2, 2021
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 2, 2021
@summer-ji-eng summer-ji-eng mentioned this pull request Nov 16, 2021
Merged
@summer-ji-eng summer-ji-eng requested a review from a team as a code owner January 5, 2022 19:52
@summer-ji-eng summer-ji-eng changed the base branch from main to drop_node_10 May 6, 2022 22:53
@summer-ji-eng summer-ji-eng deleted the branch googleapis:drop_node_10 May 9, 2022 21:06
@summer-ji-eng summer-ji-eng mentioned this pull request May 9, 2022
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@alexander-fenster alexander-fenster Awaiting requested review from alexander-fenster

Assignees

No one assigned

Labels

cla: yes This human has signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@stephenplusplus @googlebot @alexander-fenster @bcoe @summer-ji-eng @xiaozhenliu-gg5