Skip to content

refactor(sidekick/api): read-only PathBinding - #7879

Merged
coryan merged 4 commits into
googleapis:mainfrom
coryan:feat/readonly-path-binding
Nov 3, 2026
Merged

coryan merged 4 commits into
googleapis:mainfrom
coryan:feat/readonly-path-binding

Conversation

@coryan

@coryan coryan commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Make PathBinding fields private with getters and builder.

Fixes #7842

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the PathBinding struct by making its fields unexported and introducing getter and setter methods, including a query parameter getter that safely clones the map. It also introduces a new PathBindingBuilder to construct PathBinding instances and updates all references across the codebase. The review feedback recommends importing the maps package in the new builder file and cloning the map in WithQueryParameterMap to prevent external mutations.

@coryan
coryan marked this pull request as ready for review October 31, 2026 14:45
@coryan
coryan requested review from a team as code owners October 31, 2026 14:45

@amanda-tarafa amanda-tarafa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a few more comments on this one, some to possibly address immediately, some to maybe consider as part of the whole refactor.

Comment thread internal/sidekick/api/path_binding.go Resolved
Make PathBinding fields private with getters and builder.

Part of googleapis#7837
Update callers and tests to use PathBinding getters and builder.

Part of googleapis#7837

Closes googleapis#7842
Panics in (*PathBinding).SetAnnotations if the codec field is already non-nil, and ensures discovery LRO pollers and Rust tests do not share PathBinding instances across methods or subtests.
Do not overwrite an existing PathBinding.targetResource in identifyTargetResourceForBinding when it was already set via PathBindingBuilder.WithTargetResource.
@coryan
coryan force-pushed the feat/readonly-path-binding branch from 78dd11e to 51241ad Compare November 3, 2026 21:31
@coryan
coryan enabled auto-merge (squash) November 3, 2026 21:54
@coryan
coryan merged commit b39cbea into googleapis:main Nov 3, 2026
49 of 55 checks passed
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.

sidekick/api: make PathBinding read-only

2 participants