feat: write samples to file (pt3) #980
Conversation
product-auto-label
bot
added
the
samples
src/main/java/com/google/api/generator/gapic/composer/Composer.java
Outdated
Show resolved
Hide resolved
| @@ -186,6 +188,40 @@ public static List<GapicClass> generateTestClasses(GapicContext context) { | |||
| return clazzes; | |||
| } | |||
|
|
|||
| @VisibleForTesting | |||
| static List<GapicClass> composeSamples(List<GapicClass> clazzes, String protoPackage) { | |||
There was a problem hiding this comment.
This method is basically adding region tags and headers to all samples in all classes, which should probably be renamed as well. But before you do that, I have a basic question regarding why we want to do it here after samples are already mostly composed? The package info is already in the context which is passes into generateServiceClasses, so potentially we should be able to generate a full Sample in previous steps right?
There was a problem hiding this comment.
Before this step, we can generate an inline sample but not an "executable" one because inline format doesn't need a proper region tag or license. Looking at composeServiceClasses called in Generator before Writer I could parse shortname/version out before or start of composeServiceClasses() and keep passing these details around when creating Samples or I can add these details before writing the files as I'm doing. I already am iterating over the samples to add the license header here and these details aren't needed before this step.
There was a problem hiding this comment.
Yeah, it's unfortunate these info are not easily accessible and you have to pass them around. I have some refactoring ideas but should definitely not be the scope of this PR. For now, can you please rename this method to make more readable/precise?
There was a problem hiding this comment.
Makes sense and I agree it's not ideal, assuming the behaviors won't change I can test this out overall and refactor in parallel.
done, I renamed to make more readable
|
Overall looks really good. Just have one additional comment that is not directly related to this PR, can you please add more comments/Javadoc to the new model classes you added in pt1 and pt2? Especially Sample.java and RegionTag.java? I know not every model class have Javadocs currently and I should've done a better job in reviewing those earlier as well, but they would benefit new people in our team greatly. |
|
SonarCloud Quality Gate failed. |
* feat: write to files * test: update test scripts * test: goldens * feat: sample src jar * feat: package tar with samples * refactor: sample class and file names should match * test: unit goldens * test: golden IT * refactor: keep samples seperate from gapic jar * test: ComposerTest * formatting * fix test * refactor: updateSample naming * refactor: include cause with GapicWriterException * ignore test files in snippetbot check * update composeSamples name * include javadoc comments * formatting
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [org.easymock:easymock](http://easymock.org) ([source](https://togithub.com/easymock/easymock)) | `4.3` -> `5.0.1` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### 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. --- - [ ] <!-- rebase-check -->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). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4yMzcuMCIsInVwZGF0ZWRJblZlciI6IjMyLjI0MS4xMSJ9-->
](https://renovatebot.com){kind=link}
🤖 I have created a release *beep* *boop* --- ## [2.8.23](https://togithub.com/googleapis/java-core/compare/v2.8.22...v2.8.23) (2022-10-24) ### Dependencies * Update dependency io.grpc:grpc-bom to v1.50.2 ([#990](https://togithub.com/googleapis/java-core/issues/990)) ([275a212](https://togithub.com/googleapis/java-core/commit/275a212b73f6e9ea15d291f46296b7861c7a4efb)) * Update dependency org.easymock:easymock to v5 ([#980](https://togithub.com/googleapis/java-core/issues/980)) ([66a668f](https://togithub.com/googleapis/java-core/commit/66a668fd96ffe241660cb6cd2114290f4ff09697)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
go/java-sample-gen - see 'Post gapic-generator-java' section
Can easily include/not include generated samples - plan to roll out to a few before rolling out everywhere. This change won't automatically start generating samples in repos. To do so: