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

feat!: Add comment to PR #85

Merged
merged 10 commits into from
Nov 20, 2023
Merged

feat!: Add comment to PR #85

merged 10 commits into from
Nov 20, 2023

Conversation

akash1810
Copy link
Member

@akash1810 akash1810 commented Nov 17, 2023

What does this change?

In #20 a job summary was added with Riff-Raff information.

This change takes this a little further, adding a comment to the pull request with links to deploy the project in Riff-Raff. Only one comment will be made. As new builds are created, the comment is updated.

This change adds two new inputs:

  • (required) githubToken is a GitHub token with permissions to comment on a pull request.
  • (optional) commentingStage which stage to deploy to. Default of CODE.

How to test

Use the action!

Have we considered potential risks?

This should be released as a breaking change as a new required input githubToken is added.

Images

image
Draft release notes

v3.0.0

  • Add a pull request comment with quick links to deploy with Riff-Raff

BREAKING CHANGES

A new required input githubToken has been added. This needs to be scoped with pull-requests: write permission.

Before (v2)

jobs:
  CI:
    timeout-minutes: 15
    runs-on: ubuntu-latest
    steps:
      - uses: guardian/actions-riff-raff@v2
        with:
          projectName: playground::my-application
          configPath: riff-raff.yaml
          contentDirectories: |
            static-site-assets:
              - static-site/dist
            api:
              - target/application.jar

After (v3)

jobs:
  CI:
    timeout-minutes: 15
    runs-on: ubuntu-latest
    permissions: 
      pull-requests: write # <- NEW!
    steps:
      - uses: guardian/actions-riff-raff@v3 # Note the version change
        with:
          githubToken: ${{ secrets.GITHUB_TOKEN }} # <- NEW!
          projectName: playground::my-application
          configPath: riff-raff.yaml
          contentDirectories: |
            static-site-assets:
              - static-site/dist
            api:
              - target/application.jar

akash1810 added a commit to guardian/cdk-playground that referenced this pull request Nov 17, 2023
akash1810 added a commit to guardian/cdk-playground that referenced this pull request Nov 17, 2023
@akash1810 akash1810 force-pushed the aa/pr-comment branch 10 times, most recently from 0fc6281 to 7211424 Compare November 17, 2023 21:48
@akash1810 akash1810 changed the title feat: Add comment to PR feat!: Add comment to PR Nov 17, 2023
@akash1810 akash1810 marked this pull request as ready for review November 17, 2023 22:36
Extends on #20.

If the action has been started via a pull request,
add a comment to the pull request with links to deploy the build.

BREAKING CHANGE: New `githubToken` input required.
This token must have `pull_request: write` permission.
Comment on lines +28 to +29
const signature =
'_From [guardian/actions-riff-raff](https://github.com/guardian/actions-riff-raff)._';
Copy link
Member Author

@akash1810 akash1810 Nov 17, 2023

Choose a reason for hiding this comment

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

This is used to locate a previous comment to update.

Pass the PR number as an argument to `commentOnPullRequest`,
pushing logic for other events to the call site.

This makes things more logical to read too.
dist/index.js Outdated Show resolved Hide resolved
Previously if `githubToken` was not supplied we'd fail at the point of commenting.

This change fails at the start.
Comment on lines +126 to +129
### `commentingStage`
_Default: CODE_

When commenting on a pull request, which stage should be used. Typically a pre-production stage.
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not convinced this is the best variable name. Alternatives welcomed!

@rtyley
Copy link
Member

rtyley commented May 16, 2024

Only one comment will be made. As new builds are created, the comment is updated.

@akash1810 is this still intended to be the case? Unfortunately it doesn't seem to be working with the recent PRs I've seen. For instance, in this PR: https://github.com/guardian/ophan/pull/6106, there are 8 comments made by actions-riff-raff:

It's quite noisy- this happens on every single PR on Ophan - and it would be great if all these could be in the same comment, rather than 8 separate comments!

image

@akash1810
Copy link
Member Author

akash1810 commented May 16, 2024

@rtyley the action is behaving as intended - a comment is added for each Riff-Raff project an artifact has been built for. In the case of Ophan, there are 8 unique projects being built, hence 8 comments.

Folding this into a single comment sounds great! Indeed it has come up previously.

Looking at https://github.com/guardian/riffraff-platform/blob/main/packages/common/src/access.ts, there are quite a few repositories that produce more than 1 artifact, which could help prioritise implementing this.

Let's create an issue for this?

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.

None yet

4 participants