-
Notifications
You must be signed in to change notification settings - Fork 107
fix: correct bot workflow YAML for draft PR handling #985
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: main
Are you sure you want to change the base?
fix: correct bot workflow YAML for draft PR handling #985
Conversation
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.
Pull request overview
This PR fixes bot workflows to prevent them from triggering on draft pull requests, addressing issue #981.
Key Changes
- Added
ready_for_reviewevent trigger to all bot workflows to ensure they run when a draft PR is marked as ready - Added draft status checks (
github.event.pull_request.draft == false) to workflow job conditions - Updated CHANGELOG to document the fix
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| CHANGELOG.md | Documents the addition of draft PR checks to bot workflows |
| .github/workflows/pr-check-title.yml | Adds ready_for_review trigger and draft check to title validation workflow |
| .github/workflows/pr-check-changelog.yml | Adds ready_for_review trigger and draft check with workflow_dispatch handling |
| .github/workflows/merge-conflict-bot.yml | Adds ready_for_review trigger and draft check to merge conflict detection |
| .github/workflows/bot-verified-commits.yml | Adds ready_for_review trigger and draft check to commit verification |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
exploreriii
left a comment
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.
Hi @tech0priyanshu
This is really good! Nice research. Small suggestions from copilot.
Then,
Could you verify if this works?
For example, on your origin fork can you
- create a pull request for your repository eg https://github.com/YOUR_NAME/hiero_sdk_python
- merge your fixes to your origin main
then:
3. create a draft pull request to your repository, with a PR that has unverified commits but is in draft mode (and passes all other checks). Ensure it does not post unverified commit bot.
4. Change status to ready to review. Ensure it posts verified commit bot.
5. Post screenshots and a link to check it works.
Repeat to create draft pull requests for merge conflict, changelog, title check
Thank you!
|
I am reviewing this currently but we have 4 concurrent bot PRs to review. |
Signed-off-by: tech0priyanshu <priyanshuyadv101106@gmail.com>
18fc0cc to
482478f
Compare
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.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The PR workflow is functioning correctly for both draft and ready-to-review states. It skips execution when the PR is in draft mode and runs normally once it’s marked as ready for review. See the test results here: test result |
|
@exploreriii please review |
exploreriii
left a comment
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.
I think I would like to sit on this PR for a couple of days
While advanced users on draft benefit from less spam
New users on draft could benefit from the bots
We do see new users benefit from the bots at some stage
my assumption is new users will start off at ready to review PR, they don't tend to open a draft one
However, if we then start marking 'changes requested' things to 'draft' that could change
hmm
|
|
||
| - name: Harden the runner | ||
| uses: step-security/harden-runner@df199fb7be9f65074067a9eb93f12bb4c5547cf2 # v2.13.3 | ||
| with: |
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.
i don't think the changelog check is a bot?
i think its just a workflow
currently, the rule in my head is - workflows - run on draft
but bots don't post
debatable
| name: Title Check | ||
| runs-on: ubuntu-latest | ||
| if: ${{ !github.event.pull_request.base.repo.fork }} | ||
| if: ${{ !github.event.pull_request.base.repo.fork && (github.event_name == 'workflow_dispatch' || github.event.pull_request.draft == false) }} |
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.
same with this i think?
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.
but we do have bot workflows and bot verified commits which i would argue should only run on ready for review
debatable
|
@manishdait Because we will have new starters that are now working from draft mode so will get no feedback until ready for review On one hand, they can help a user progress faster in draft mode |
|
If we’re auto-moving PRs to draft when changes are requested. In that case, I’d actually prefer that only the GitHub workflows run in draft, and we disable the bots entirely for (merge conflict bot, verified commit bot, workflow bot) This will avoids unnecessary spamming when they are working with their changes and The workflows will still surface actual build/test failures, which is enough guidance at the draft stage. |
|
but then we have a new starter -> makes a ready for review PR -> workflow fails -> i have to manualyl tell them what to improve (bot doesn't run, I would have to do that)? |
Description:
Related issue(s):
Fixes #981
Checklist