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.

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

chore: initiate library generation if Dockerfile is changed #2878

Merged
merged 5 commits into from
Jun 12, 2024

Conversation

JoeWang1127
Copy link
Collaborator

@JoeWang1127 JoeWang1127 commented Jun 11, 2024

In this PR:

  • Initiate generation process if the Dockerfile is changed.
  • Move docker build and mvn install in shell script to speed up the workflow run if no changes in the generation_config.yaml and Dockerfile.

Example run: https://github.com/JoeWang1127/sdk-platform-java/actions/runs/9473714657/job/26101875644?pr=8

@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Jun 11, 2024
@JoeWang1127 JoeWang1127 changed the title Chore/change gen trigger chore: initiate library generation if Dockerfile is changed Jun 11, 2024
@JoeWang1127 JoeWang1127 marked this pull request as ready for review June 11, 2024 22:40
@JoeWang1127 JoeWang1127 requested a review from a team as a code owner June 11, 2024 22:40
exit 0
fi
# copy generation configuration from target branch to current branch.
git show "${target_branch}":"${generation_config}" > "${baseline_generation_config}"
config_diff=$(diff "${generation_config}" "${baseline_generation_config}" || true)

# install generator locally since we're using a SNAPSHOT version.
mvn -V -B -ntp clean install -DskipTests
Copy link
Collaborator

@blakeli0 blakeli0 Jun 12, 2024

Choose a reason for hiding this comment

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

I'm not sure we want to move mvn install and docker build to the scripts here. It does provide better encapsulation, but people often want to run the scripts directly in local, doing this here would significantly slow things down. I think it's OK to keep them in the workflow at this moment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This workflow will run on every pull request and it'll take ~5 mins to build the image and install generator only to discovery that there's no config/Dockerfile change or no code change at last.

The trade-off here is to speed up checks in pull requests.

On the other hand, I don't think this script is good for local development since it will checkout branches and create pull request.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This workflow will run on every pull request and it'll take ~5 mins to build the image and install generator only to discovery that there's no config/Dockerfile change or no code change at last.

That's a good point, agreed.

I don't think this script is good for local development

Looks like it does not. Can we add more info regarding local development in a follow up task? For example, what are the steps if I want to generate a random library with a SNAPSHOT version of generator and/or docker images? We have some good stuff already in DEVELOPMENT.md, and we can add more on top of it.

@JoeWang1127 JoeWang1127 requested a review from blakeli0 June 12, 2024 17:46
Copy link

Quality Gate Passed Quality Gate passed for 'gapic-generator-java-root'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link

Quality Gate Passed Quality Gate passed for 'java_showcase_integration_tests'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@JoeWang1127 JoeWang1127 merged commit 74f20a2 into main Jun 12, 2024
46 of 47 checks passed
@JoeWang1127 JoeWang1127 deleted the chore/change-gen-trigger branch June 12, 2024 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants