Skip to content

Conversation

@vam-google
Copy link
Contributor

This PR depends on the corresponding one in gax: googleapis/gax-java#1680

This PR depends on the corresponding one in gax: googleapis/gax-java#1680
@vam-google vam-google requested a review from a team May 13, 2022 22:37
@vam-google vam-google requested a review from a team as a code owner May 13, 2022 22:37
@vam-google vam-google requested review from blakeli0 and eaball35 May 13, 2022 22:38
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication


public abstract String pattern();

public abstract List<String> additionalPatterns();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to use List here? Can we use Set? Does order matter? Do duplicates matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think technically duplicates are allowed even though they will be useless. Bindings come in list from proto file descriptor, so keeping them as list here.


// This method tries to parse all named segments from pattern and sort in natual order
// e.g. /v1beta1/{table_name=tests/*}/{routing_id=instances/*}/** -> (routing_id, table_name)
public static Set<String> getPattenBindings(String pattern) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch! Thanks for correcting my mistake!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I actually thought it was my typo, only now realized that it was the class you created =).

@vam-google vam-google merged commit ce58c18 into googleapis:main May 19, 2022
suztomo pushed a commit that referenced this pull request Dec 16, 2022
This PR depends on the corresponding one in gax: googleapis/gax-java#1680
suztomo pushed a commit that referenced this pull request Mar 21, 2023
🤖 I have created a release *beep* *boop*
---


### Updating meta-information for bleeding-edge SNAPSHOT release.

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
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.

2 participants