chore: add a composite action for library generation #3173
Conversation
.github/scripts/action.yaml
Outdated
| @@ -0,0 +1,51 @@ | |||
| name: Copy library generation script | |||
| description: Copy library generation shell script to the calling workflow | |||
There was a problem hiding this comment.
| description: Copy library generation shell script to the calling workflow | |
| description: Copies library generation shell script to the calling workflow |
Just a quick nit: In general, descriptions should not be imperative statements but rather assertions of what the file/object/etc does.
There was a problem hiding this comment.
I updated the name and description.
.github/scripts/action.yaml
Outdated
| run: | | ||
| if [[ "${GITHUB_REPOSITORY}" != "${REPO_FULL_NAME}" ]]; then | ||
| echo "This PR comes from a fork. Skip library generation." | ||
| exit 0 |
There was a problem hiding this comment.
is it possible that this exit 0 just means for the runner that the step finished successfully?
There was a problem hiding this comment.
Do you think we should fail this step?
There was a problem hiding this comment.
I think any way of stopping the workflow without throwing an error would work fine. I added this comment because I think this exit 0 will be considered as a successful step and the next step would then start. Maybe using GITHUB_OUTPUT along with if in the following steps may do the job (SO)?
There was a problem hiding this comment.
I added a separate job to determine whether to run the library generation.
If the pull request comes from a fork, the generation will be skipped.
|
The following error occurs if I removed the local install. |
.github/scripts/action.yaml
Outdated
| description: the tag of hermetic build image | ||
| required: false | ||
| token: | ||
| description: PAT |
There was a problem hiding this comment.
Personal access token.
There was a problem hiding this comment.
Can we use the full name instead of abbreviations since it's not common? See style guide.
There was a problem hiding this comment.
Changed to full name.
| # run hermetic code generation docker image. | ||
| docker run \ | ||
| --rm \ | ||
| -u "$(id -u):$(id -g)" \ | ||
| -v "$(pwd):${workspace_name}" \ | ||
| -v "$HOME"/.m2:/home/.m2 \ | ||
| -e GENERATOR_VERSION="${generator_version}" \ | ||
| -v "${m2_folder}":/home/.m2 \ |
| fetch-depth: 0 | ||
| token: ${{ secrets.CLOUD_JAVA_BOT_TOKEN }} | ||
| - name: Generate changed libraries | ||
| - name: Determine whether the pull request comes from a fork |
There was a problem hiding this comment.
I keep this step to prevent this workflow runs in a fork.
The workflow will fail if it's a fork and it can prevent us from setting this workflow as required, e.g., renovate PR will be blocked by the failure.
|
|
| echo "This PR comes from a fork. Skip library generation." | ||
| echo "SHOULD_RUN=false" >> $GITHUB_ENV | ||
| else | ||
| echo "SHOULD_RUN=true" >> $GITHUB_ENV |
There was a problem hiding this comment.
Is there a way to exit the workflow early so we don't have to pass this env to all the subsequent steps?
There was a problem hiding this comment.
Looks like another way of skipping steps is creating a separate job and make the generation job depends on that, just like what we've done in unit/integration tests.
I'll keep it as-is now.
In this PR: - Add a composite action to execute library generation shell script. - Separate repo specific logic, e.g., installing modules, building images, etc., in the generation shell script. An example workflow in google-cloud-java: googleapis/google-cloud-java#11117
In this PR:
An example workflow in google-cloud-java: googleapis/google-cloud-java#11117