-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
ci(prereleaser): verify user membership for running workflow #6734
Conversation
Signed-off-by: Prashant Shahi <[email protected]>
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
1 similar comment
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Reviewed everything up to 13f17ff in 42 seconds
More details
- Looked at
48
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. .github/workflows/prereleaser.yaml:45
- Draft comment:
Consider adding error handling for the GitHub API call to handle potential network issues or invalid tokens. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The code already has implicit error handling through the jq command's default value "not-member". If the API call fails for any reason (network, invalid token, etc.), the script will exit with code 1, which is the desired behavior for a GitHub Action - fail the workflow if verification fails. The comment suggests adding explicit error handling, but the current implementation is actually sufficient for the use case.
I might be overlooking specific GitHub Actions best practices for error handling. There could be value in having more descriptive error messages for debugging.
While more detailed error messages could be helpful, the current implementation is sufficient for the primary goal of failing the workflow when verification fails. The GitHub Actions logs will show the curl command output for debugging.
The comment should be deleted because the code already handles errors appropriately for its purpose, and the suggested change isn't clearly necessary or beneficial.
2. .github/workflows/prereleaser.yaml:21
- Draft comment:
Avoid using thecomponent/index.tsx
file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. This comment applies to the overall project structure if applicable. - Reason this comment was not posted:
Confidence changes required:50%
The code is well-structured and follows the rules provided. No issues with design tokens, inline styles, or ClickHouseReader interface violations.
Workflow ID: wflow_68Clh1SthFCl79Qc
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 69b51ab in 28 seconds
More details
- Looked at
51
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. .github/workflows/prereleaser.yaml:49
- Draft comment:
The team name used here is 'releaser', but the PR description mentions 'releasers'. Ensure the team name is correct to avoid logical errors. - Reason this comment was not posted:
Comment did not seem useful.
2. .github/workflows/prereleaser.yaml:52
- Draft comment:
Avoid using thecomponent/index.tsx
file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. This is applicable to thesignoz
job definition. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_b3CqcjtqFf8tPGaj
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Signed-off-by: Prashant Shahi <[email protected]>
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on a9ab179 in 16 seconds
More details
- Looked at
103
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. .github/workflows/prereleaser.yaml:22
- Draft comment:
Ensure that the referencefeat/github
is correct and intended for production use. If this is a feature branch, it might not be stable for production. - Reason this comment was not posted:
Comment did not seem useful.
2. .github/workflows/releaser.yaml:25
- Draft comment:
Ensure that the referencefeat/github
is correct and intended for production use. If this is a feature branch, it might not be stable for production. - Reason this comment was not posted:
Marked as duplicate.
3. .github/workflows/prereleaser.yaml:22
- Draft comment:
Avoid using thecomponent/index.tsx
file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. Ensure this is not used elsewhere in the project. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_mposJFoS9FXawixf
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Signed-off-by: Prashant Shahi <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 7690fe0 in 13 seconds
More details
- Looked at
27
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. .github/workflows/releaser.yaml:22
- Draft comment:
The current logic only distinguishes between 'patch' and 'minor' releases. Consider adding logic to handle 'major' releases as well. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
2. .github/workflows/prereleaser.yaml:22
- Draft comment:
Ensure that the workflow file paths and references are correct and up-to-date with the main branch to avoid any issues during execution. - Reason this comment was not posted:
Confidence changes required:33%
The code changes in the PR do not violate any of the specified rules. The changes are related to GitHub Actions workflows and do not involve any React components, ClickHouseReader interface, or file structure issues. The use of external workflows and secrets is appropriate for the context.
Workflow ID: wflow_TIKAmBrZq7oWDIri
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
Summary
Important
Adds user membership verification to
prereleaser.yaml
and release type detection toreleaser.yaml
.verify
job inprereleaser.yaml
to check if the user is a 'releasers' team member before triggering the workflow.actions/create-github-app-token@v1
for GitHub API requests.signoz
job dependent onverify
job inprereleaser.yaml
.detect
job inreleaser.yaml
to determine release type and pass it tocharts
job.This description was created by for 7690fe0. It will automatically update as commits are pushed.