feat(github): point to ref/repo from PR HEAD, add/remove labels #10
Conversation
Signed-off-by: Prashant Shahi <[email protected]>
Signed-off-by: Prashant Shahi <[email protected]>
WalkthroughThis pull request introduces two new GitHub Actions workflows, Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/github-remove-label.yaml (1)
6-10: 🛠️ Refactor suggestionRemove default value for required input.
Same issue as in github-add-label workflow: the
PRIMUS_REFinput has a redundant default value while being required.🧰 Tools
🪛 actionlint (1.7.4)
9-9: input "PRIMUS_REF" of workflow_call event has the default value "main", but it is also required. if an input is marked as required, its default value will never be used
(events)
🧹 Nitpick comments (1)
.github/workflows/github-add-label.yaml (1)
56-60: Consider escaping potential spaces in label names.Label names might contain spaces, and while the current quoting handles it, consider using
"${GH_LABEL_NAME}"for consistency with shell best practices.$MAKE github-add-label \ GITHUB_ARGS="-t ${{ steps.token.outputs.token }} \ -r ${GH_REPOSITORY} \ -i ${GH_ISSUE_NUMBER} \ - -l '${GH_LABEL_NAME}'" + -l \"${GH_LABEL_NAME}\""
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/github-add-label.yaml(1 hunks).github/workflows/github-remove-label.yaml(1 hunks).github/workflows/go-fmt.yaml(1 hunks).github/workflows/go-lint.yaml(1 hunks).github/workflows/go-test.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/github-add-label.yaml
9-9: input "PRIMUS_REF" of workflow_call event has the default value "main", but it is also required. if an input is marked as required, its default value will never be used
(events)
.github/workflows/github-remove-label.yaml
9-9: input "PRIMUS_REF" of workflow_call event has the default value "main", but it is also required. if an input is marked as required, its default value will never be used
(events)
🔇 Additional comments (4)
.github/workflows/go-fmt.yaml (1)
28-30: LGTM! Consistent implementation across workflows.The addition of
refandrepositoryparameters to the self-checkout step correctly ensures that the formatting is applied to the PR's HEAD state..github/workflows/go-lint.yaml (1)
28-30: LGTM! Consistent with go-fmt workflow.The changes ensure linting is performed on the PR's HEAD state.
.github/workflows/go-test.yaml (1)
36-38: LGTM! Consistent with other go workflows.The changes ensure tests are run against the PR's HEAD state while preserving the workflow's test-specific configuration.
.github/workflows/github-remove-label.yaml (1)
51-60: Apply the same improvements as github-add-label workflow.The same considerations about issue number availability and label name quoting apply here.
| PRIMUS_REF: | ||
| description: 'The primus ref to checkout.' | ||
| required: true | ||
| default: 'main' | ||
| type: string |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove default value for required input.
The PRIMUS_REF input is marked as required but also has a default value. This is redundant as the default value will never be used for required inputs.
PRIMUS_REF:
description: 'The primus ref to checkout.'
required: true
- default: 'main'
type: string📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| PRIMUS_REF: | |
| description: 'The primus ref to checkout.' | |
| required: true | |
| default: 'main' | |
| type: string | |
| PRIMUS_REF: | |
| description: 'The primus ref to checkout.' | |
| required: true | |
| type: string |
🧰 Tools
🪛 actionlint (1.7.4)
9-9: input "PRIMUS_REF" of workflow_call event has the default value "main", but it is also required. if an input is marked as required, its default value will never be used
(events)
| env: | ||
| GH_REPOSITORY: ${{ github.repository }} | ||
| GH_ISSUE_NUMBER: ${{ github.event.issue.number }} | ||
| GH_LABEL_NAME: ${{ inputs.GITHUB_LABEL_NAME }} |
There was a problem hiding this comment.
💡 Codebase verification
The workflow needs context validation or parameter adjustment
The workflow is designed as a reusable workflow (workflow_call trigger) but assumes github.event.issue.number will always be available. This assumption is unsafe as:
- Pull requests use
github.event.pull_request.numberinstead - Other event contexts might not have an issue number
- As a reusable workflow, it could be called from various event contexts
Consider either:
- Adding the number as an input parameter
- Adding context validation
- Handling different event types (issues/PRs) appropriately
🔗 Analysis chain
Verify issue number availability in event context.
The workflow assumes github.event.issue.number will be available, but this might not be true for all trigger contexts (e.g., PRs vs Issues).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the workflow is used in any other workflow files
# to understand the trigger contexts
echo "Checking workflow usage contexts..."
rg -l "uses: ./.github/workflows/github-add-label.yaml" .
Length of output: 140
Script:
#!/bin/bash
# First, let's look at the workflow file content
echo "=== Workflow File Content ==="
cat .github/workflows/github-add-label.yaml
echo -e "\n=== Searching for workflow references with different patterns ==="
rg -l "github-add-label" .
echo -e "\n=== Looking for workflow files that might use this ==="
fd "workflow" .github/
Length of output: 2060
Script:
#!/bin/bash
# Search for workflow files that might call this workflow
echo "=== Searching for workflow calls ==="
rg -l "workflow.*github-add-label" .github/workflows/
# Look for the actual workflow call pattern
echo -e "\n=== Searching for specific workflow call patterns ==="
rg "uses:.*github-add-label" .github/workflows/
Length of output: 311
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/github-add-label.yaml (1)
25-27: Consider parameterizing the makefile path.The makefile path is hardcoded in the MAKE environment variable. Consider making it configurable through an input parameter to improve maintainability if the repository structure changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/github-add-label.yaml(1 hunks).github/workflows/github-remove-label.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/github-add-label.yaml
9-9: input "PRIMUS_REF" of workflow_call event has the default value "main", but it is also required. if an input is marked as required, its default value will never be used
(events)
.github/workflows/github-remove-label.yaml
9-9: input "PRIMUS_REF" of workflow_call event has the default value "main", but it is also required. if an input is marked as required, its default value will never be used
(events)
🔇 Additional comments (6)
.github/workflows/github-add-label.yaml (4)
6-10: Remove default value for required input.The
PRIMUS_REFinput is marked as required but also has a default value. This is redundant as the default value will never be used for required inputs.PRIMUS_REF: description: 'The primus ref to checkout.' required: true - default: 'main' type: string🧰 Tools
🪛 actionlint (1.7.4)
9-9: input "PRIMUS_REF" of workflow_call event has the default value "main", but it is also required. if an input is marked as required, its default value will never be used
(events)
29-35: LGTM! Good security practices.The job configuration follows security best practices by:
- Using the latest stable runner
- Setting minimal required permissions
36-52: LGTM! Secure token handling.The implementation follows security best practices:
- Uses GitHub App token instead of personal access token
- Properly scopes token to repository owner
- Securely passes token to checkout steps
55-65: LGTM! Safe command construction.The label addition step:
- Properly handles spaces in label names
- Uses environment variables securely
- Passes token safely
.github/workflows/github-remove-label.yaml (2)
6-10: Remove default value for required input.Same issue as in
github-add-label.yaml: ThePRIMUS_REFinput is marked as required but also has a default value.🧰 Tools
🪛 actionlint (1.7.4)
9-9: input "PRIMUS_REF" of workflow_call event has the default value "main", but it is also required. if an input is marked as required, its default value will never be used
(events)
11-15: Add validation for issue/PR number availability.Same issue as in
github-add-label.yaml: The default value assumes event context that might not be available.
| GITHUB_ISSUE_NUMBER: | ||
| description: 'The issue number to add the label to.' | ||
| required: false | ||
| type: string | ||
| default: ${{ github.event.pull_request.number || github.event.issue.number }} |
There was a problem hiding this comment.
💡 Codebase verification
The review comment is valid and needs attention.
The workflow is a reusable workflow that relies on the calling workflow's event context. The current implementation could fail if:
- The calling workflow's context doesn't have either
pull_request.numberorissue.number - The make command receives an empty issue number
Recommendations:
- Make
GITHUB_ISSUE_NUMBERrequired (preferred) - Or add explicit validation before the make command
- Document that this workflow expects to be called from issue/PR-related event contexts
🔗 Analysis chain
Add validation for issue/PR number availability.
The default value assumes either github.event.pull_request.number or github.event.issue.number will be available, but this might not be true for all workflow trigger contexts. Consider:
- Adding explicit validation for the number
- Making this input required
- Documenting the expected event contexts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the workflow is used in contexts where issue/PR numbers might be unavailable
echo "Checking workflow usage contexts..."
rg -l "uses: ./.github/workflows/github-add-label.yaml" .
Length of output: 140
Script:
#!/bin/bash
# Try different path formats and check the workflow file itself
echo "Checking workflow references with different path formats..."
rg -l "github-add-label\.ya?ml" .
echo -e "\nChecking the workflow file contents..."
cat .github/workflows/github-add-label.yaml
echo -e "\nChecking if this is a new workflow..."
git log --diff-filter=A -- .github/workflows/github-add-label.yaml
Length of output: 2219
| name: github-remove-label | ||
|
|
||
| on: | ||
| workflow_call: | ||
| inputs: | ||
| PRIMUS_REF: | ||
| description: 'The primus ref to checkout.' | ||
| required: true | ||
| default: 'main' | ||
| type: string | ||
| GITHUB_ISSUE_NUMBER: | ||
| description: 'The issue number to remove the label from.' | ||
| required: false | ||
| type: string | ||
| default: ${{ github.event.pull_request.number || github.event.issue.number }} | ||
| GITHUB_LABEL_NAME: | ||
| description: 'The name of the label to remove.' | ||
| required: true | ||
| type: string | ||
|
|
||
| defaults: | ||
| run: | ||
| shell: bash | ||
|
|
||
| env: | ||
| PRIMUS_HOME: .primus | ||
| MAKE: make --no-print-directory --makefile=.primus/src/make/main.mk | ||
|
|
||
| jobs: | ||
| signoz: | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: 'read' | ||
| id-token: 'write' | ||
| steps: | ||
| - name: self-checkout | ||
| uses: actions/checkout@v4 | ||
| - id: token | ||
| name: github-token-gen | ||
| uses: actions/create-github-app-token@v1 | ||
| with: | ||
| app-id: ${{ secrets.PRIMUS_APP_ID }} | ||
| private-key: ${{ secrets.PRIMUS_PRIVATE_KEY }} | ||
| owner: ${{ github.repository_owner }} | ||
| - name: primus-checkout | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: signoz/primus | ||
| ref: ${{ inputs.PRIMUS_REF }} | ||
| path: .primus | ||
| token: ${{ steps.token.outputs.token }} | ||
| - name: info | ||
| run: | | ||
| $MAKE info | ||
| - name: remove-label | ||
| env: | ||
| GH_REPOSITORY: ${{ github.repository }} | ||
| GH_ISSUE_NUMBER: ${{ inputs.GITHUB_ISSUE_NUMBER }} | ||
| GH_LABEL_NAME: ${{ inputs.GITHUB_LABEL_NAME }} | ||
| run: | | ||
| $MAKE github-remove-label \ | ||
| GITHUB_ARGS="-t ${{ steps.token.outputs.token }} \ | ||
| -r ${GH_REPOSITORY} \ | ||
| -i ${GH_ISSUE_NUMBER} \ | ||
| -l '${GH_LABEL_NAME}'" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider creating a reusable workflow template.
This workflow is nearly identical to github-add-label.yaml. Consider:
- Creating a template workflow that accepts an
actioninput parameter - Using the template for both add and remove label operations
- Reducing maintenance overhead and potential inconsistencies
Example implementation:
# .github/workflows/github-label-template.yaml
name: github-label-template
on:
workflow_call:
inputs:
ACTION:
description: 'The label action to perform (add/remove).'
required: true
type: string
# ... other inputs remain same ...
# ... rest of the configuration remains same ...
- name: label-action
env:
GH_REPOSITORY: ${{ github.repository }}
GH_ISSUE_NUMBER: ${{ inputs.GITHUB_ISSUE_NUMBER }}
GH_LABEL_NAME: ${{ inputs.GITHUB_LABEL_NAME }}
run: |
$MAKE github-${{ inputs.ACTION }}-label \
GITHUB_ARGS="-t ${{ steps.token.outputs.token }} \
-r ${GH_REPOSITORY} \
-i ${GH_ISSUE_NUMBER} \
-l '${GH_LABEL_NAME}'"Then use it like:
# .github/workflows/github-add-label.yaml
name: github-add-label
on:
workflow_call:
# ... inputs remain same except ACTION ...
jobs:
add-label:
uses: ./.github/workflows/github-label-template.yaml
with:
ACTION: 'add'
# ... pass other inputs ...🧰 Tools
🪛 actionlint (1.7.4)
9-9: input "PRIMUS_REF" of workflow_call event has the default value "main", but it is also required. if an input is marked as required, its default value will never be used
(events)
Signed-off-by: Prashant Shahi <[email protected]>
Features
Summary by CodeRabbit
New Features
github-add-labelfor automatically adding labels to GitHub issuesgithub-remove-labelfor removing labels from GitHub issuesImprovements
go-fmt,go-lint,go-test) to dynamically check out specific pull request branches and repositories