Skip to content

Conversation

@zhumin8
Copy link
Contributor

@zhumin8 zhumin8 commented Nov 16, 2022

Two changes:

@conventional-commit-lint-gcf
Copy link

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@zhumin8 zhumin8 changed the title fix(spring): remove maven-jar-plugin from pom string. fix(spring): add parent and remove maven-jar-plugin from pom string. Nov 16, 2022
@zhumin8 zhumin8 marked this pull request as ready for review November 16, 2022 19:26
@zhumin8 zhumin8 requested a review from a team November 16, 2022 19:26
@zhumin8 zhumin8 requested a review from a team as a code owner November 16, 2022 19:26
@emmileaf
Copy link
Contributor

A somewhat relevant thought that doesn't have to be addressed in this PR: would it make sense to have pom.xml live as a template file under spring-cloud-gcp/generator, instead of generated by SpringWriter.java?

I was looking into replacing the content string with some sort of templating here, but It seems like the generated file is already pretty much a template (only parsed apiShortName comes from the generator, and we can probably find a better substitute for that in post-processing). Not sure if I'm missing anything here though - @zhumin8 what do you think?

Copy link
Contributor

@diegomarquezp diegomarquezp left a comment

Choose a reason for hiding this comment

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

LGTM
Following @emmileaf 's line, a good patter would be that of rules_gapic if we want to use templates for the pom.

@diegomarquezp
Copy link
Contributor

parent pom coordinate is not expected to change. #1086 can be an improvement on top of this hardcoded parent.

Maybe we can just close #1086

@zhumin8
Copy link
Contributor Author

zhumin8 commented Nov 17, 2022

On the template note, haven't looked closely at @diegomarquezp's suggestion yet, but can we do something similar to client_grpcrest.gradle.tmpl?

@zhumin8 zhumin8 merged commit 0f7d580 into autoconfig-gen-draft2 Nov 17, 2022
@zhumin8 zhumin8 deleted the fix-pom-string branch November 17, 2022 18:32
@zhumin8 zhumin8 added the spring pr that's related to spring code gen, intend to merge into autoconfig-gen-draft2 branch. label Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

spring pr that's related to spring code gen, intend to merge into autoconfig-gen-draft2 branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants