feat: update generated samples to executable format #874

eaball35 · 2021-11-30T20:58:21Z

b/207550139, b/207550144

go/java-sample-gen
go/java-sample-gen-bugs

codecov · 2021-11-30T21:14:01Z

Codecov Report

Merging #874 (8a7927c) into main (6af0e18) will increase coverage by 0.08%.
The diff coverage is 98.97%.

@@            Coverage Diff             @@
##             main     #874      +/-   ##
==========================================
+ Coverage   87.84%   87.93%   +0.08%     
==========================================
  Files         153      156       +3     
  Lines       15966    16068     +102     
  Branches     1155     1155              
==========================================
+ Hits        14025    14129     +104     
  Misses       1601     1601              
+ Partials      340      338       -2

Impacted Files	Coverage Δ
...i/generator/engine/writer/ImportWriterVisitor.java	`94.56% <ø> (+1.61%)`	⬆️
...enerator/gapic/composer/samplecode/SampleUtil.java	`93.75% <93.75%> (ø)`
.../composer/samplecode/ExecutableSampleComposer.java	`98.48% <98.48%> (ø)`
...ser/common/AbstractServiceClientClassComposer.java	`98.62% <100.00%> (ø)`
...r/common/AbstractServiceSettingsClassComposer.java	`97.72% <100.00%> (ø)`
...mmon/AbstractServiceStubSettingsClassComposer.java	`96.21% <100.00%> (ø)`
...or/gapic/composer/samplecode/ExecutableSample.java	`100.00% <100.00%> (ø)`
...or/gapic/composer/samplecode/SampleCodeWriter.java	`92.85% <100.00%> (+5.35%)`	⬆️
...er/samplecode/ServiceClientSampleCodeComposer.java	`99.29% <100.00%> (+<0.01%)`	⬆️
...omposer/samplecode/SettingsSampleCodeComposer.java	`98.66% <100.00%> (-0.06%)`	⬇️
... and 1 more

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 6af0e18...8a7927c. Read the comment docs.

amanda-tarafa

LGTM on how the resulting samples are looking, this is great.

meltsufin

@amanda-tarafa @eaball35
I see that this only updates inline samples, and the single-file samples as described in the design are not part of this PR.
I'm a little surprised to see essentially the whole executable sample file with imports and a wrapping application class inline. Was that the intent of the original design all along? Do you feel like this might be adding too much boilerplate repetitive code to the javadoc samples?

amanda-tarafa · 2021-12-06T14:59:43Z

@meltsufin I'm starting a thread internally to discuss more in-depth, but basically, these aspects are up for decision for each language, as they know their community better.

chanseokoh

Just went over the top part of the PR. Will take another look later.

.../java/com/google/api/generator/gapic/composer/common/AbstractServiceClientClassComposer.java

chanseokoh · 2021-12-06T19:19:16Z

src/main/java/com/google/api/generator/gapic/composer/samplecode/ExecutableSample.java

+    this.sampleVariableAssignments = sampleVariableAssignments;
+    this.sampleBody = sampleBody;


Just to confirm: is there any chance that the input List instances may be modified?

chanseokoh · 2021-12-06T19:19:45Z

src/main/java/com/google/api/generator/gapic/composer/samplecode/ExecutableSample.java

+    return sampleName;
+  }
+
+  public List<AssignmentExpr> getSampleVariableAssignments() {


Likewise, any chance that the output List object may be modified?

src/main/java/com/google/api/generator/gapic/composer/samplecode/ExecutableSampleComposer.java

chanseokoh · 2021-12-06T21:42:23Z

src/main/java/com/google/api/generator/gapic/composer/samplecode/SampleCodeWriter.java

+    JavaWriterVisitor visitor = new JavaWriterVisitor();
+    classDefinition.accept(visitor);
+    // Escape character "@" in the markdown code block <pre>{@code...} tags.
+    return visitor.write().replaceAll("@", "{@literal @}");


Since write(List<Statement>) does this too, will this end up double-escaping @? If not, I actually think this is unnecessary? And should we format code here instead of write(List<Statement>)?

src/main/java/com/google/api/generator/gapic/composer/samplecode/SampleUtil.java

chanseokoh · 2021-12-06T22:07:36Z

src/main/java/com/google/api/generator/gapic/composer/samplecode/SampleUtil.java

+  public static MethodInvocationExpr systemOutPrint(String content) {
+    return composeSystemOutPrint(ValueExpr.withValue(StringObjectValue.withValue(content)));
+  }
+
+  public static MethodInvocationExpr systemOutPrint(VariableExpr variableExpr) {
+    return composeSystemOutPrint(variableExpr.toBuilder().setIsDecl(false).build());
+  }
+
+  static MethodInvocationExpr composeSystemOutPrint(Expr content) {


These method are not called by non-test code. Let's remove them.

Agreed, however I created them because eventually I will definitely want to use them in the samples to handle the responses. I was just getting a little ahead of myself implementing these now.

That's actually what I suspected. Let's add them in the future when they are needed.

...java/com/google/api/generator/gapic/composer/samplecode/ServiceClientSampleCodeComposer.java

eaball35 · 2021-12-14T23:51:40Z

Forking as new branch as we decided to not make inline snippets "executable" format but instead link to full example if users wants it - #882

chore: pin versions of certifi and google-resumable-media Source-Link: googleapis/synthtool@09c4fcd Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:264c6d5da60ff1684fbdd2b268d6a3ffca2038246e0948a06f15ca0c3cf28ce8 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

…874) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.protobuf:protobuf-bom](https://developers.google.com/protocol-buffers/) ([source](https://togithub.com/protocolbuffers/protobuf)) | `3.21.2` -> `3.21.3` | [![age](https://badges.renovateapi.com/packages/maven/com.google.protobuf:protobuf-bom/3.21.3/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.protobuf:protobuf-bom/3.21.3/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.protobuf:protobuf-bom/3.21.3/compatibility-slim/3.21.2)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.protobuf:protobuf-bom/3.21.3/confidence-slim/3.21.2)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>protocolbuffers/protobuf</summary> ### [`v3.21.3`](https://togithub.com/protocolbuffers/protobuf/compare/v3.21.2...v3.21.3) [Compare Source](https://togithub.com/protocolbuffers/protobuf/compare/v3.21.2...v3.21.3) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-core).

🤖 I have created a release *beep* *boop* --- ## [2.8.3](googleapis/java-core@v2.8.2...v2.8.3) (2022-07-26) ### Dependencies * update dependency com.google.api-client:google-api-client-bom to v2 ([#868](googleapis/java-core#868)) ([fe7991d](googleapis/java-core@fe7991d)) * update dependency com.google.api:gax-bom to v2.18.4 ([#864](googleapis/java-core#864)) ([d4a8501](googleapis/java-core@d4a8501)) * update dependency com.google.api:gax-bom to v2.18.5 ([#876](googleapis/java-core#876)) ([e2c0c13](googleapis/java-core@e2c0c13)) * update dependency com.google.api.grpc:proto-google-common-protos to v2.9.2 ([#870](googleapis/java-core#870)) ([adf5af4](googleapis/java-core@adf5af4)) * update dependency com.google.api.grpc:proto-google-iam-v1 to v1.5.2 ([#865](googleapis/java-core#865)) ([5dad1ea](googleapis/java-core@5dad1ea)) * update dependency com.google.auth:google-auth-library-bom to v1.8.1 ([#856](googleapis/java-core#856)) ([bb58609](googleapis/java-core@bb58609)) * update dependency com.google.http-client:google-http-client-bom to v1.42.2 ([#871](googleapis/java-core#871)) ([341b04e](googleapis/java-core@341b04e)) * update dependency com.google.protobuf:protobuf-bom to v3.21.3 ([#874](googleapis/java-core#874)) ([efe8c28](googleapis/java-core@efe8c28)) * update dependency com.google.protobuf:protobuf-bom to v3.21.4 ([#877](googleapis/java-core#877)) ([25bea6c](googleapis/java-core@25bea6c)) * update dependency io.grpc:grpc-bom to v1.48.0 ([#873](googleapis/java-core#873)) ([8bfee63](googleapis/java-core@8bfee63)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

eaball35 added 2 commits November 29, 2021 19:07

feat: ExecutableSampleComposer

444a91f

feat: make current samples executable samples

fce9bda

eaball35 requested review from miraleung and a team as code owners November 30, 2021 20:58

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 30, 2021

product-auto-label bot added the samples Issues that are directly related to samples. label Nov 30, 2021

eaball35 closed this Nov 30, 2021

eaball35 reopened this Nov 30, 2021

eaball35 marked this pull request as draft November 30, 2021 21:00

chore: include license headers

cc25bb8

eaball35 added 7 commits November 30, 2021 14:59

tests + cleanup

23665b2

include generic exceptions

9846013

tests: update integration goldens

c292b3b

chore: formatting

78e13aa

use example package instead and include needed imports

066fc4d

cleanup

36d1985

formatting

e8e7312

eaball35 marked this pull request as ready for review December 2, 2021 16:36

cleanup

15782a6

eaball35 assigned vam-google, lesv, meltsufin and amanda-tarafa Dec 2, 2021

meltsufin assigned chanseokoh Dec 2, 2021

amanda-tarafa approved these changes Dec 3, 2021

View reviewed changes

meltsufin reviewed Dec 3, 2021

View reviewed changes

Neenu1995 added the release-please:force-run To run release-please label Dec 6, 2021

release-please bot removed the release-please:force-run To run release-please label Dec 6, 2021

Merge branch 'main' into java-sample-gen

e65a06b

chanseokoh reviewed Dec 6, 2021

View reviewed changes

eaball35 added 2 commits December 14, 2021 14:35

small fixes

fb15ad1

fix import

8a7927c

eaball35 closed this Dec 14, 2021

chanseokoh deleted the java-sample-gen branch December 15, 2021 15:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: update generated samples to executable format #874

feat: update generated samples to executable format #874

eaball35 commented Nov 30, 2021 •

edited

Loading

codecov bot commented Nov 30, 2021 •

edited

Loading

amanda-tarafa left a comment

meltsufin left a comment

amanda-tarafa commented Dec 6, 2021

chanseokoh left a comment

chanseokoh Dec 6, 2021

chanseokoh Dec 6, 2021

chanseokoh Dec 6, 2021

chanseokoh Dec 6, 2021

eaball35 Dec 14, 2021

chanseokoh Dec 14, 2021

eaball35 commented Dec 14, 2021 •

edited

Loading

		this.sampleVariableAssignments = sampleVariableAssignments;
		this.sampleBody = sampleBody;

feat: update generated samples to executable format #874

feat: update generated samples to executable format #874

Conversation

eaball35 commented Nov 30, 2021 • edited Loading

codecov bot commented Nov 30, 2021 • edited Loading

Codecov Report

amanda-tarafa left a comment

Choose a reason for hiding this comment

meltsufin left a comment

Choose a reason for hiding this comment

amanda-tarafa commented Dec 6, 2021

chanseokoh left a comment

Choose a reason for hiding this comment

chanseokoh Dec 6, 2021

Choose a reason for hiding this comment

chanseokoh Dec 6, 2021

Choose a reason for hiding this comment

chanseokoh Dec 6, 2021

Choose a reason for hiding this comment

chanseokoh Dec 6, 2021

Choose a reason for hiding this comment

eaball35 Dec 14, 2021

Choose a reason for hiding this comment

chanseokoh Dec 14, 2021

Choose a reason for hiding this comment

eaball35 commented Dec 14, 2021 • edited Loading

eaball35 commented Nov 30, 2021 •

edited

Loading

codecov bot commented Nov 30, 2021 •

edited

Loading

eaball35 commented Dec 14, 2021 •

edited

Loading