Skip to content

Extensible client options - #1263

Merged
chingor13 merged 7 commits into
googleapis:masterfrom
chingor13:system-parameters
Jun 18, 2019
Merged

Extensible client options#1263
chingor13 merged 7 commits into
googleapis:masterfrom
chingor13:system-parameters

Conversation

@chingor13

Copy link
Copy Markdown
Collaborator

Introduces the builder pattern for these common configuration options so that it is extensible in the future.
Add ability to configure requestReason and userAgent.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 22, 2019
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Jun 4, 2019
chingor13 added 4 commits June 5, 2019 16:41
…lient.

Introduces the builder pattern for these common configuration options so that it is extensible in the future.
@chingor13
chingor13 force-pushed the system-parameters branch from 62e32a4 to 1678fef Compare June 5, 2019 23:41
@sduskis sduskis added needs work This is a pull request that needs a little love. and removed 🚨 This issue needs some love. labels Jun 6, 2019
@chingor13
chingor13 marked this pull request as ready for review June 17, 2019 17:35
@chingor13
chingor13 requested a review from a team June 17, 2019 17:35
@chingor13 chingor13 removed the needs work This is a pull request that needs a little love. label Jun 17, 2019
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Jun 17, 2019
@sduskis

sduskis commented Jun 17, 2019

Copy link
Copy Markdown

Should this class use Auto-Value?

@chingor13

Copy link
Copy Markdown
Collaborator Author

Should this class use Auto-Value?

We certainly could. We're not using autovalue yet for any of the other builders (or at all). It might make sense to come back with a refactoring PR to convert the all builders to use autovalue (if it's even possible). Otherwise, we will have a mix of several manually defined builders and one autovalue builder. WDYT?

@chingor13

chingor13 commented Jun 17, 2019

Copy link
Copy Markdown
Collaborator Author

@sduskis Stubbed out an autovalue implementation of CommonGoogleOptions in #1326. I'm not sure we can shim autovalue into the existing classes as they are already part of a class where users instantiate the classes directly.

@chingor13

Copy link
Copy Markdown
Collaborator Author

I'm not sold on trying to shoehorn autovalue in here. If we were starting from scratch, it would make a lot of sense. We cannot switch many of these implementations over to autovalue as they already have public constructors (including the CommonGoogleClientRequestInitializer)

@chingor13
chingor13 merged commit 6000d9f into googleapis:master Jun 18, 2019
@chingor13
chingor13 deleted the system-parameters branch June 18, 2019 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement. 🚨 This issue needs some love.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants