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

Comment pull-request with Build Scan links in Publish action #4

Merged
merged 6 commits into from
Sep 26, 2023

Conversation

jprinet
Copy link
Member

@jprinet jprinet commented Sep 14, 2023

Description

This PR adds published Build Scan links as comments to the pull-request. This is required as there is no easy way to correlate the Workflow execution (not using a pull_request trigger) with the original pull request.
This happens by modifying the publish composite action.

Testing

Same as for the publish action, no testing is provided

Implementation Details

The github-token is added as action inputs to allow commenting the pull-request.

Preview

Screenshot 2023-09-26 at 2 34 46 PM

Shortcomings

The Build Scan link is retrieved by grepping the Maven console output which is not elegant.
This could be revisited once the Maven init action will be alive.

Comment

This PR has the jprinet/add-publish-maven-build-scan-action as base for clarity

@jprinet jprinet requested a review from bigdaz September 14, 2023 16:35
@jprinet jprinet force-pushed the jprinet/comment-pr-with-build-scan-links branch from 5203859 to 3699e84 Compare September 14, 2023 16:37
@jprinet jprinet force-pushed the jprinet/add-publish-maven-build-scan-action branch 15 times, most recently from 9741932 to ffe1f1a Compare September 19, 2023 13:26
@jprinet jprinet force-pushed the jprinet/comment-pr-with-build-scan-links branch 7 times, most recently from 5f8d425 to cb12371 Compare September 19, 2023 15:25
@jprinet jprinet force-pushed the jprinet/add-publish-maven-build-scan-action branch from ffe1f1a to ff543ce Compare September 19, 2023 15:37
@jprinet jprinet force-pushed the jprinet/comment-pr-with-build-scan-links branch 5 times, most recently from 78f4889 to 4b288d4 Compare September 19, 2023 15:54
@jprinet jprinet force-pushed the jprinet/comment-pr-with-build-scan-links branch from 4b288d4 to 90c17d7 Compare September 19, 2023 16:01
Copy link
Member

@bigdaz bigdaz left a comment

Choose a reason for hiding this comment

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

I think we should simply render the build scan URL directly, rather than obfuscating it with "Link N". Otherwise, LGTM.

scanLink=$(grep -A1 "Publishing build scan..." build.out | tail -n 1 | sed 's/\[INFO\] //')
if [[ ! -z "$scanLink" ]]
then
scanLinks="${scanLinks},[Link $((publishedScans++))]($scanLink)"
Copy link
Member

Choose a reason for hiding this comment

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

I think these links would be more useful if they were raw so the user could see the URL. If we have a nice human-readable text for the build then that would be good, but the "Link 1" text just obfuscates things.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're totally right 👍

@jprinet jprinet force-pushed the jprinet/add-publish-maven-build-scan-action branch from 5649d11 to 599b9f2 Compare September 26, 2023 09:50
Base automatically changed from jprinet/add-publish-maven-build-scan-action to main September 26, 2023 10:09
@jprinet jprinet force-pushed the jprinet/comment-pr-with-build-scan-links branch from ce120a8 to ba6bf14 Compare September 26, 2023 10:28
@jprinet jprinet force-pushed the jprinet/comment-pr-with-build-scan-links branch from ba6bf14 to f352754 Compare September 26, 2023 12:25
@jprinet jprinet marked this pull request as ready for review September 26, 2023 12:36
@jprinet jprinet merged commit b9eb35b into main Sep 26, 2023
3 checks passed
@jprinet jprinet deleted the jprinet/comment-pr-with-build-scan-links branch September 26, 2023 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants