Combine TOS acceptance and publication in a single workflow #6

jprinet · 2023-09-29T14:31:13Z

This PR creates a top-level composite action which aggregates (load / verify / publish) to offer a single step workflow on consumer side.

This modification requires the contributor-assistant to cope with a non pull_request event which is addressed by:
contributor-assistant/github-action#142

jprinet · 2023-10-02T08:20:10Z

maven-build-scan/action.yml

+        build-workflow-filename: ${{ inputs.build-workflow-filename }}
+        pr-comment-tos-acceptance-request: ${{ inputs.pr-comment-tos-acceptance-request }}
+    - name: Verify Terms of Service acceptance
+      uses: gradle/github-actions/terms-of-service-acceptance/run@v0.1


I am not super happy with the need for a fully qualified name of the composite action as the tag needs to be updated on each new release (which could easily be forgotten).

The alternative would be to checkout the github-actions code in the calling workflow to allow calling composite actions with relative paths, this looks worse. though

The common solution to this is to use a version alias (like @v1) that always points to the latest release. This is done via a tag that is force pushed on each release.

If we do that, then this file won't need to be updated on each release, but it will still contain the fully-qualified path to the action (which I think is OK).

Sounds good 👍

jprinet · 2023-10-02T08:20:43Z

maven-build-scan/publish/action.yml

@@ -10,39 +10,18 @@ inputs:
    required: false
  develocity-allow-untrusted:
    description: 'Develocity allow-untrusted flag'
-    default: 'false'
+    required: true


I set the defaults on the top level action only

bigdaz

LGTM. I can't help but think that this will be a lot simpler if we re-implement in TypeScript. But I like the minimally intrusive requirements on the project.

bigdaz · 2023-10-02T19:28:05Z

maven-build-scan/action.yml

+  signature-branch:
+    description: 'Git branch where the signature file will be stored'
+    default: ${{ github.event.repository.default_branch }}
+  signature-location:


This is probably better named "tos-acceptance-file", since it will always be the path to a file, and it records the "tos acceptance".

bigdaz · 2023-10-02T19:30:07Z

maven-build-scan/action.yml

+  pr-comment-tos-acceptance-validation:
+    description: 'pull-request comment added when Terms of Service are accepted'
+    default: 'All Contributors have accepted Develocity Terms of Service.'
+  signature-branch:


I wonder if "signature" is the best prefix here. This is the branch (and file) where we record TOS-acceptance. Maybe "tos-acceptance-file" and "tos-acceptance-file-branch" would be more self-explanatory as input names.

bigdaz · 2023-10-02T19:33:37Z

maven-build-scan/load/action.yml

+        )
+        && github.event_name != 'workflow_run'
+      run: |
+        echo "Skipping Github event"


This message won't make much sense in the logs on it's own, I don't think. What event is being skipped and why? (Also, maybe this should be logged as DEBUG)

I adjusted the message, I kept it in INFO as I think adding some feedback makes the output clearer

bigdaz · 2023-10-02T19:36:59Z

README.md

+          tos-location: 'https://foo.bar/tos.html'
+          develocity-url: 'https://<MY_DEVELOCITY_URL>'
+          develocity-access-key: ${{ secrets.<DEVELOCITY_ACCESS_KEY> }}
+```


YES! This is all so much simpler from the user point of view, and will make it easier for us to evolve the implementation under the covers.

bigdaz · 2023-10-02T19:39:17Z

README.md

+      - name: Publish Build Scan
+        uses: gradle/github-actions/maven-build-scan@v1.0
+        with:
+          build-workflow-filename: 'build.yml'


It's unfortunate that this is requires, but I guess that when this workflow is triggered via issue_comment we have no way of knowing what workflow file might have uploaded build scan artifacts.

bigdaz · 2023-10-02T19:43:34Z

maven-build-scan/load/action.yml

+      with:
+        pr: ${{ github.event.issue.number }}
+        workflow_conclusion: success
+        workflow: ${{ inputs.build-workflow-filename }}


I trust that these 3 inputs together mean that we'll only load build scan data that was uploaded by build-workflow_filename AND it was triggered by a pull_request event from the specified PR. (We don't want to upload build scans saved by the workflow for a different PR!

Yes, this step is called on issue_comment event only, meaning github.event.issue.number represents the current PR

jprinet · 2023-10-03T07:28:15Z

LGTM. I can't help but think that this will be a lot simpler if we re-implement in TypeScript. But I like the minimally intrusive requirements on the project.

Agreed, I'll add the TS based implementation into the backlog

- rename signature to Terms of Service - Adjust Check event trigger log message - Adjust inner action tags

jprinet · 2023-11-23T13:49:56Z

Closing as the following requirement could not be met

This modification requires the contributor-assistant to cope with a non pull_request event which is addressed by:
contributor-assistant/github-action#142

Unify publication workflow

4a2b29a

jprinet requested a review from bigdaz September 29, 2023 15:07

jprinet force-pushed the jprinet/unified-workflow branch 8 times, most recently from aed9c0a to 661f0d1 Compare October 2, 2023 08:07

Add aggregate action

0eb3d02

jprinet force-pushed the jprinet/unified-workflow branch from 661f0d1 to 0eb3d02 Compare October 2, 2023 08:12

jprinet marked this pull request as ready for review October 2, 2023 08:17

jprinet commented Oct 2, 2023

View reviewed changes

jprinet force-pushed the jprinet/unified-workflow branch 15 times, most recently from 2c2ebde to 80b7c5e Compare October 2, 2023 09:48

jprinet force-pushed the jprinet/unified-workflow branch from 80b7c5e to 4c3716e Compare October 2, 2023 09:50

Add test action

d89c9c8

jprinet force-pushed the jprinet/unified-workflow branch from 4c3716e to d89c9c8 Compare October 2, 2023 09:51

bigdaz reviewed Oct 2, 2023

View reviewed changes

CR follow-up

3d31e85

- rename signature to Terms of Service - Adjust Check event trigger log message - Adjust inner action tags

jprinet force-pushed the jprinet/unified-workflow branch from 4e84692 to 3d31e85 Compare October 3, 2023 07:33

jprinet closed this Nov 23, 2023

jprinet deleted the jprinet/unified-workflow branch January 30, 2024 10:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Combine TOS acceptance and publication in a single workflow #6

Combine TOS acceptance and publication in a single workflow #6

jprinet commented Sep 29, 2023 •

edited

Loading

jprinet Oct 2, 2023

bigdaz Oct 2, 2023

jprinet Oct 3, 2023

jprinet Oct 2, 2023

bigdaz left a comment

bigdaz Oct 2, 2023

bigdaz Oct 2, 2023

bigdaz Oct 2, 2023

jprinet Oct 3, 2023

bigdaz Oct 2, 2023

bigdaz Oct 2, 2023

bigdaz Oct 2, 2023

jprinet Oct 3, 2023

jprinet commented Oct 3, 2023

jprinet commented Nov 23, 2023

Combine TOS acceptance and publication in a single workflow #6

Combine TOS acceptance and publication in a single workflow #6

Conversation

jprinet commented Sep 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bigdaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jprinet commented Oct 3, 2023

jprinet commented Nov 23, 2023

jprinet commented Sep 29, 2023 •

edited

Loading