edk12564 commented on code in PR #5437:
URL: https://github.com/apache/fineract/pull/5437#discussion_r2757399138
##########
.github/workflows/pr-one-commit-per-user-check.yml:
##########
@@ -0,0 +1,57 @@
+name: Fineract PR One Commit Per User Check
+
+
+on:
+ pull_request:
+ types: [opened, reopened, synchronize]
+
+
+permissions:
+ pull-requests: write
+
+
+jobs:
+ verify-commits:
+ name: Validate One Commit Per User
+ runs-on: ubuntu-latest
+ timeout-minutes: 1
+ steps:
+ - name: Verify Commit Policy
+ id: check
+ env:
+ GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ run: |
+
+ commits=$(gh api "repos/$REPO/pulls/$PR_NUMBER/commits") || { echo
"::error::GitHub API request failed"; exit 1; }
+
+ if echo "$commits" | jq -e '.[] | select(.author == null)' >
/dev/null; then
+ echo "null_authors=true" >> $GITHUB_OUTPUT
+ echo "::error::Some commits have a git email that is not linked to
a GitHub account. Please ensure your git email matches one of your GitHub
Account emails."
+ exit 1
+ fi
+
+ user_ids=$(echo "$commits" | jq -r '.[] | select(.author.type !=
"Bot") | .author.id')
+ if echo "$user_ids" | sort | uniq -d | grep -q .; then
+ echo "multiple_commits=true" >> $GITHUB_OUTPUT
+ echo "::error::Multiple commits from the same author have been
detected."
+ exit 1
+ fi
+
+ echo "Success: Each author has exactly one commit."
+
+ - name: Comment on PR
+ if: failure()
+ env:
+ GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Review Comment:
I did a bit of research, and I agree with your view. The Stale workflow has
a lot of merits, and I will look into adding to the what we have. To summarize
though, this is what I found.
1 - The current implementation needs permissions for it to run properly.
Stale.yml uses the main repo's context, which allows it to run without manual
approval.
On this note, I found something we could use. If we use pull_request_target
instead of pull_request, we could run our workflows with base repo permissions.
I am reading about it here:
https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#pull_request_target.
The main issue for this seems to be that this raises security concerns about
running code without prior approval. But I think this might be fine considering
we are only looking at the commit repository, PR number, and userIds. We don't
actually execute any foreign code. Let me know what you think on this one!
2 - I also see what you mean about running a cronjob. I think this is a good
idea as well. However, if we want to run on a cronjob to handle older PRs, the
issue is that we probably shouldn't spam comments in case of users abandoning
their PR. I think we should implement a max alert counter, check the most
recent alert, or maybe we won't produce warnings if the no one has committed in
the past day or two? I will look into figuring out our options.
What do you think on those points?
##########
.github/workflows/pr-one-commit-per-user-check.yml:
##########
@@ -0,0 +1,57 @@
+name: Fineract PR One Commit Per User Check
+
+
+on:
+ pull_request:
+ types: [opened, reopened, synchronize]
+
+
+permissions:
+ pull-requests: write
+
+
+jobs:
+ verify-commits:
+ name: Validate One Commit Per User
+ runs-on: ubuntu-latest
+ timeout-minutes: 1
+ steps:
+ - name: Verify Commit Policy
+ id: check
+ env:
+ GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ run: |
+
+ commits=$(gh api "repos/$REPO/pulls/$PR_NUMBER/commits") || { echo
"::error::GitHub API request failed"; exit 1; }
+
+ if echo "$commits" | jq -e '.[] | select(.author == null)' >
/dev/null; then
+ echo "null_authors=true" >> $GITHUB_OUTPUT
+ echo "::error::Some commits have a git email that is not linked to
a GitHub account. Please ensure your git email matches one of your GitHub
Account emails."
+ exit 1
+ fi
+
+ user_ids=$(echo "$commits" | jq -r '.[] | select(.author.type !=
"Bot") | .author.id')
+ if echo "$user_ids" | sort | uniq -d | grep -q .; then
+ echo "multiple_commits=true" >> $GITHUB_OUTPUT
+ echo "::error::Multiple commits from the same author have been
detected."
+ exit 1
+ fi
+
+ echo "Success: Each author has exactly one commit."
+
+ - name: Comment on PR
+ if: failure()
+ env:
+ GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Review Comment:
I did a bit of research, and I agree with your view. The Stale workflow has
a lot of merits, and I will look into adding to what we have. To summarize
though, this is what I found.
1 - The current implementation needs permissions for it to run properly.
Stale.yml uses the main repo's context, which allows it to run without manual
approval.
On this note, I found something we could use. If we use pull_request_target
instead of pull_request, we could run our workflows with base repo permissions.
I am reading about it here:
https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#pull_request_target.
The main issue for this seems to be that this raises security concerns about
running code without prior approval. But I think this might be fine considering
we are only looking at the commit repository, PR number, and userIds. We don't
actually execute any foreign code. Let me know what you think on this one!
2 - I also see what you mean about running a cronjob. I think this is a good
idea as well. However, if we want to run on a cronjob to handle older PRs, the
issue is that we probably shouldn't spam comments in case of users abandoning
their PR. I think we should implement a max alert counter, check the most
recent alert, or maybe we won't produce warnings if the no one has committed in
the past day or two? I will look into figuring out our options.
What do you think on those points?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
