Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[DO NOT REVIEW][Merged In #573][CloseSoon]Use resource name class to construct String Default value #577

Closed
wants to merge 7 commits into from
Closed

[DO NOT REVIEW][Merged In #573][CloseSoon]Use resource name class to construct String Default value #577

wants to merge 7 commits into from

Conversation

summer-ji-eng
Copy link
Contributor

Default value on a string-type method argument of resource name is constructed by resources name class.
Use

   * try (ConfigClient configClient = ConfigClient.create()) {
   *   LogSinkName sinkName = LogSinkName.ofProjectSinkName("[PROJECT]", "[SINK]");
   *   configClient.deleteSink(sinkName.toString());
   * }

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Dec 1, 2020
miraleung
miraleung suggested changes Dec 2, 2020
View reviewed changes
src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java
@@ -123,6 +134,13 @@ static Expr createDefaultValue(Field f, boolean useExplicitInitTypeInGenerics) {
}

if (f.type().equals(TypeNode.STRING)) {
if (useSampleCode
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not pass in the ResourceName-typed field instead if the field is a String? Then we don't need to change DefaultValueComposer at all.

src/main/java/com/google/api/generator/gapic/composer/MethodSampleCodeHelperComposer.java Outdated
DefaultValueComposer.createDefaultValue(arg, resourceNames)))
arg -> {
Expr defaultValueExpr =
DefaultValueComposer.createDefaultValue(arg, resourceNames, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Here: If arg is String-typed and it has a matching resource name, pass that arg in instead.

src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java
}

static Expr createDefaultValue(
Field f, Map<String, ResourceName> resourceNames, Boolean useSampleCode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the motivation for using the boxed type?

src/main/java/com/google/api/generator/gapic/composer/MethodSampleCodeHelperComposer.java
arguments.stream()
.map(arg -> methodArgVarExprMap.get(arg.name()))
.collect(Collectors.toList());
createMethodArgVarExprs(arguments, methodArgVarExprMap, resourceNames);
Copy link
Contributor

Choose a reason for hiding this comment

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

PTAL at my comments on splitting out the VarExprs in #573.

@summer-ji-eng
Copy link
Contributor Author

Thanks for valuable comments. Make sense to me. Instead of pass boolean value into DefaultValueComposer, I will use your advice and merged in #573 .

@summer-ji-eng summer-ji-eng changed the title [fix]Use resource name class to construct String Default value [Merged In #573][CloseSoon]Use resource name class to construct String Default value Dec 2, 2020
@summer-ji-eng summer-ji-eng changed the title [Merged In #573][CloseSoon]Use resource name class to construct String Default value [DO NOT REVIEW][Merged In #573][CloseSoon]Use resource name class to construct String Default value Dec 2, 2020
@summer-ji-eng
Copy link
Contributor Author

Merge this PR into #573

@summer-ji-eng summer-ji-eng deleted the string_default_value_1.1 branch January 19, 2021 23:18
suztomo pushed a commit that referenced this pull request Mar 21, 2023
@renovate-bot
build(deps): update dependency com.google.cloud:google-cloud-shared-c…
cb1752f 
…onfig to v1.2.6 (#577)
suztomo pushed a commit that referenced this pull request Mar 21, 2023
@renovate-bot
deps: update dependency com.google.guava:guava-bom to v31 (#577)
a72fd39
suztomo pushed a commit that referenced this pull request Mar 21, 2023
@release-please
chore: release 2.1.8 (#582)
a311e92 
🤖 I have created a release \*beep\* \*boop\*
---
### [2.1.8](https://www.github.com/googleapis/java-core/compare/v2.1.7...v2.1.8) (2021-10-15)


### Dependencies

* update dependency com.google.api-client:google-api-client-bom to v1.32.2 ([#593](https://www.github.com/googleapis/java-core/issues/593)) ([5471efd](https://www.github.com/googleapis/java-core/commit/5471efda5271fc21df2ace74d4aa59dfff610a53))
* update dependency com.google.api:api-common to v2.0.4 ([#585](https://www.github.com/googleapis/java-core/issues/585)) ([3063023](https://www.github.com/googleapis/java-core/commit/3063023e34e10deef2a65d82a39a36d03296f145))
* update dependency com.google.api:api-common to v2.0.5 ([#589](https://www.github.com/googleapis/java-core/issues/589)) ([354ff97](https://www.github.com/googleapis/java-core/commit/354ff97f777f7b151744fc542bb43cb9caa3ba4a))
* update dependency com.google.api:gax-bom to v2.6.0 ([#594](https://www.github.com/googleapis/java-core/issues/594)) ([123ebcc](https://www.github.com/googleapis/java-core/commit/123ebcc927585ff42e4b6cdbde26deb527b45621))
* update dependency com.google.api.grpc:proto-google-common-protos to v2.6.0 ([#578](https://www.github.com/googleapis/java-core/issues/578)) ([cfd5d51](https://www.github.com/googleapis/java-core/commit/cfd5d51c5e82690a2eac536fbd75e3c789c52e36))
* update dependency com.google.api.grpc:proto-google-iam-v1 to v1.1.3 ([#574](https://www.github.com/googleapis/java-core/issues/574)) ([92c2a59](https://www.github.com/googleapis/java-core/commit/92c2a599d916d0103ad152c380c1f4c1a8bcfd81))
* update dependency com.google.api.grpc:proto-google-iam-v1 to v1.1.4 ([#584](https://www.github.com/googleapis/java-core/issues/584)) ([6984a40](https://www.github.com/googleapis/java-core/commit/6984a407a1b8f3466d0d695465f988d9138c95f6))
* update dependency com.google.api.grpc:proto-google-iam-v1 to v1.1.5 ([#587](https://www.github.com/googleapis/java-core/issues/587)) ([30e3d99](https://www.github.com/googleapis/java-core/commit/30e3d99f893da41af9ae3ac22356bed2bddc1fa8))
* update dependency com.google.api.grpc:proto-google-iam-v1 to v1.1.6 ([#590](https://www.github.com/googleapis/java-core/issues/590)) ([e7446c8](https://www.github.com/googleapis/java-core/commit/e7446c8475ee339abcf3f77bb993689501235014))
* update dependency com.google.auth:google-auth-library-bom to v1.2.0 ([#581](https://www.github.com/googleapis/java-core/issues/581)) ([3390141](https://www.github.com/googleapis/java-core/commit/3390141b28f90690956222c0d6661d502f91c706))
* update dependency com.google.auth:google-auth-library-bom to v1.2.1 ([#591](https://www.github.com/googleapis/java-core/issues/591)) ([7c4a658](https://www.github.com/googleapis/java-core/commit/7c4a6580faed1201d7e136d40f71a8805235e8ba))
* update dependency com.google.guava:guava-bom to v31 ([#577](https://www.github.com/googleapis/java-core/issues/577)) ([a72fd39](https://www.github.com/googleapis/java-core/commit/a72fd395d4eded744fb110dd01bac12906d7c564))
* update dependency com.google.http-client:google-http-client-bom to v1.40.1 ([#588](https://www.github.com/googleapis/java-core/issues/588)) ([a9402ff](https://www.github.com/googleapis/java-core/commit/a9402ff2d38eb220988550c04f2c27975ca9c6b9))
* update dependency com.google.protobuf:protobuf-bom to v3.18.1 ([#583](https://www.github.com/googleapis/java-core/issues/583)) ([6ccb52f](https://www.github.com/googleapis/java-core/commit/6ccb52f7386abbc2232178678523befc7e4b7e16))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miraleung miraleung miraleung requested changes

@xiaozhenliu-gg5 xiaozhenliu-gg5 Awaiting requested review from xiaozhenliu-gg5

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@summer-ji-eng @miraleung