Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions .github/workflows/comment-pr.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Description: This workflow is triggered when the `receive-pr` workflow completes to post suggestions on the PR.
# Since this pull request has write permissions on the target repo, we should **NOT** execute any untrusted code.
# https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
---
name: comment-pr

on:
workflow_run:
workflows: ["receive-pr"]
types:
- completed

jobs:
post-suggestions:
# https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#running-a-workflow-based-on-the-conclusion-of-another-workflow
if: ${{ github.event.workflow_run.conclusion == 'success' }}
runs-on: ubuntu-latest
permissions:
pull-requests: write
env:
# https://docs.github.com/en/actions/reference/authentication-in-a-workflow#permissions-for-the-github_token
ACCESS_TOKEN: ${{ secrets.GITHUB_TOKEN }}
timeout-minutes: 10
steps:
- uses: actions/checkout@v4
with:
ref: ${{github.event.workflow_run.head_branch}}
repository: ${{github.event.workflow_run.head_repository.full_name}}

# Download the patch
- uses: actions/download-artifact@v4
with:
name: patch
github-token: ${{ secrets.GITHUB_TOKEN }}
run-id: ${{ github.event.workflow_run.id }}
- name: Apply patch
run: |
git apply git-diff.patch --allow-empty
rm git-diff.patch

# Download the PR number
- uses: actions/download-artifact@v4
with:
name: pr_number
github-token: ${{ secrets.GITHUB_TOKEN }}
run-id: ${{ github.event.workflow_run.id }}
- name: Read pr_number.txt
run: |
PR_NUMBER=$(cat pr_number.txt)
echo "PR_NUMBER=$PR_NUMBER" >> $GITHUB_ENV
rm pr_number.txt

# Post suggestions as a comment on the PR
- uses: googleapis/code-suggester@v4
Copy link
Member

Choose a reason for hiding this comment

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

@timtebeek , do you have visual examples in web on how this workflow works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes sure! The blog above links to this pull request that has a few of such resolved comments

Copy link
Member

Choose a reason for hiding this comment

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

image

this approach if very noisy, we tried it with Sonar, long time ago, it produce too much comments, and PRs started to load slow and we need to close PR and open it again. When you have pre-junior contributors as majority in projects, this is not very good approach. Just failed CI and contributors open failed job to see suggestions is better.

in slow traffic project and experienced contributors it will work.

we can try it in this project and see how it will go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I find it works best when the suggestions are immediately applied, but I do like how contextualized they are and that they don't break flow ideally: just press a button and move on. I hope that'll be your experience as well.

with:
command: review
pull_number: ${{ env.PR_NUMBER }}
git_dir: '.'
66 changes: 66 additions & 0 deletions .github/workflows/receive-pr.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# Description: This workflow runs OpenRewrite recipes against opened pull request and upload the patch.
# Since this pull request receives untrusted code, we should **NOT** have any secrets in the environment.
# https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
---
name: receive-pr

on:
pull_request:
types: [opened, synchronize]
branches:
- main

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

defaults:
run:
shell: bash

env:
MAVEN_OPTS: -Xmx8g
GRADLE_OPTS: -Dorg.gradle.jvmargs='-Xmx8g'

jobs:
upload-patch:
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
- uses: actions/checkout@v4
with:
ref: ${{github.event.pull_request.head.ref}}
repository: ${{github.event.pull_request.head.repo.full_name}}
- name: Set up JDK 17
uses: actions/setup-java@v4
with:
distribution: temurin
java-version: 17
cache: 'maven'

# Capture the PR number
# https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#using-data-from-the-triggering-workflow
- name: Create pr_number.txt
run: echo "${{ github.event.number }}" > pr_number.txt
- uses: actions/upload-artifact@v4
with:
name: pr_number
path: pr_number.txt
- name: Remove pr_number.txt
run: rm -f pr_number.txt

# Execute recipes
- name: Apply OpenRewrite recipes
run: |
mvn --batch-mode \
checkstyle:check -Dcheckstyle.failOnViolation=false \
rewrite:run -Drewrite.activeRecipes=org.checkstyle.recipes.OpenRewriteRecipeBestPractices

# Capture the diff
- name: Create patch
run: |
git diff | tee git-diff.patch
- uses: actions/upload-artifact@v4
with:
name: patch
path: git-diff.patch
34 changes: 17 additions & 17 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,23 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
<version>${maven.checkstyle.plugin.version}</version>
<configuration>
<includeResources>false</includeResources>
<includeTestResources>false</includeTestResources>
<includeTestSourceDirectory>true</includeTestSourceDirectory>
<configLocation>
https://raw.githubusercontent.com/checkstyle/checkstyle/checkstyle-${checkstyle.version}/config/checkstyle-checks.xml
</configLocation>
<propertiesLocation>config/checkstyle.properties</propertiesLocation>
<failOnViolation>true</failOnViolation>
<logViolationsToConsole>true</logViolationsToConsole>
<maxAllowedViolations>0</maxAllowedViolations>
<violationSeverity>error</violationSeverity>
<outputFileFormat>xml</outputFileFormat>
<outputFile>
${project.build.directory}/checkstyle/checkstyle-report.xml
</outputFile>
</configuration>
<dependencies>
<dependency>
<groupId>com.puppycrawl.tools</groupId>
Expand All @@ -146,23 +163,6 @@
<goals>
<goal>check</goal>
</goals>
<configuration>
<includeResources>false</includeResources>
<includeTestResources>false</includeTestResources>
<includeTestSourceDirectory>true</includeTestSourceDirectory>
<configLocation>
https://raw.githubusercontent.com/checkstyle/checkstyle/checkstyle-${checkstyle.version}/config/checkstyle-checks.xml
</configLocation>
<propertiesLocation>config/checkstyle.properties</propertiesLocation>
<failOnViolation>true</failOnViolation>
<logViolationsToConsole>true</logViolationsToConsole>
<maxAllowedViolations>0</maxAllowedViolations>
<violationSeverity>error</violationSeverity>
<outputFileFormat>xml</outputFileFormat>
<outputFile>
${project.build.directory}/checkstyle/checkstyle-report.xml
</outputFile>
</configuration>
</execution>
</executions>
</plugin>
Expand Down
Loading