Skip to content
This repository has been archived by the owner on Sep 27, 2023. It is now read-only.

fix: enable complex resource ids in instantiate() #159

Merged
merged 1 commit into from
Jul 1, 2020

Conversation

miraleung
Copy link
Contributor

Fixes #154.

@miraleung miraleung requested a review from vam-google June 30, 2020 19:54
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 30, 2020
@codecov
Copy link

codecov bot commented Jun 30, 2020

Codecov Report

Merging #159 into master will increase coverage by 0.33%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #159      +/-   ##
============================================
+ Coverage     64.09%   64.43%   +0.33%     
- Complexity      171      174       +3     
============================================
  Files            14       14              
  Lines           713      717       +4     
  Branches        118      120       +2     
============================================
+ Hits            457      462       +5     
+ Misses          220      219       -1     
  Partials         36       36              
Impacted Files Coverage Δ Complexity Δ
...java/com/google/api/pathtemplate/PathTemplate.java 69.66% <100.00%> (+0.50%) 117.00 <0.00> (+3.00)

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 39612f1...a178f1a. Read the comment docs.

Copy link
Contributor

@vam-google vam-google left a comment

Choose a reason for hiding this comment

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

LGTM

@devchas
Copy link

devchas commented Jul 1, 2020

I just tested this out with a local build, and it seems to have fixed the problem. Thank you for the quick fix here! Any idea when this might make it into the next release. As mentioned in #154, we have another large body of work which depends on this feature, with a hard deadline in mid-July.

@miraleung miraleung merged commit 955b8a7 into master Jul 1, 2020
@miraleung miraleung deleted the fix/instantiate branch July 1, 2020 18:13
Truth.assertThat(match.get("foo")).isEqualTo("foo1");
Truth.assertThat(match.get("bar")).isEqualTo("bar2");
variables = template.vars();
System.out.println("DEL: vars: " + variables);
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be here

.containsExactly("foo", "bar", "zone_a", "zone_b", "zone_c", "cell1", "cell2");

pattern = "projects/{foo}_{bar}/zones/*";
template = PathTemplate.create(pattern);
Copy link
Contributor

Choose a reason for hiding this comment

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

local variables probably shouldn't be reused

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to return PathTemplate with complex resource identifiers?
5 participants