-
Notifications
You must be signed in to change notification settings - Fork 45.8k
dx(platform): Add format-on-command workflow #10037
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
base: dev
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for auto-gpt-docs-dev canceled.
|
Here's the code health analysis summary for commits Analysis Summary
|
✅ Deploy Preview for auto-gpt-docs canceled.
|
Thank you for submitting this PR to add a format-on-command workflow! A few things need to be addressed before we can merge:
This is a useful addition that will help streamline our development process, so I'm looking forward to seeing these adjustments! |
Thank you for adding this format-on-command workflow! This will be a great addition to improve the developer experience when working with the platform codebase. However, before we can merge this PR, please complete the checklist in the PR description. Specifically:
Since this is a GitHub workflow, your test plan might involve:
Please update the PR description with these details so we can proceed with the review. |
Thanks for adding this format-on-command workflow! This will be helpful for streamlining the PR process. Before we can merge this, please complete the checklist by:
A test plan for this type of workflow might include:
Since this workflow will be modifying code automatically on PRs, we want to make sure it's thoroughly tested before merging. |
Thanks for adding this useful developer experience improvement! The workflow for formatting on command will be a great addition to streamline code quality maintenance. A couple of items need to be addressed before this can be merged:
Since this workflow runs on PR comments and modifies code, a test plan is particularly important to ensure it works as expected and doesn't cause any issues. Your implementation looks solid overall - I like how it checks if the commenter is the PR author or a maintainer before running the formatters. |
- name: Checkout PR source | ||
uses: actions/checkout@v4 | ||
with: | ||
repository: ${{ steps.pr.outputs.head_repo }} | ||
ref: ${{ steps.pr.outputs.head_ref }} |
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.
@ntindle @Bentlybro is this dangerous to do in a workflow with repo write permissions?
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.
Claude says yes it is, and there is no clear-cut mitigation
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.
Yes in my opinion I’d say this is potentially dangerous, mainly due to the direct commit and push to user-controlled branches.
Maybe there is a better way we can do this with a github bot instead of a cl action? or something else.
Changes
!format
is added by the author or a maintainerChecklist