fix: correct bot workflow YAML for draft PR handling#985
fix: correct bot workflow YAML for draft PR handling#985tech0priyanshu wants to merge 2 commits intohiero-ledger:mainfrom
Conversation
There was a problem hiding this comment.
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.
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.
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.
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.
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.
same with this i think?
There was a problem hiding this comment.
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. |
This comment was marked as outdated.
This comment was marked as outdated.
|
Let's say if bots only run on draft:
The issue I see is:
Let's say if bots only run on ready to review and not on draft:
The issue I see is:
Overall, not clear if there is a net benefit |
|
@tech0priyanshu This is awesome work! I reviewed the workflow changes and the implementation itself looks solid! I agree this makes sense for the “bot-style” workflows (verified commits, merge conflict bot), where avoiding noise during draft is beneficial. One nuance worth calling out (already touched on in the thread) is that some of these are more workflows than bots and there may be value in still running those during draft for early feedback, especially for new contributors. That said, from a technical perspective the changes look good to me. I think the remaining question is a product/maintainer decision on desired behavior. Additionally, For PR formatting and requirements please use the link below. If you have any questions please feel free to reach out to me, ( @aceppaluni ) or our amazing team! We are always happy to assist! |
|
yap you are correct @aceppaluni then this is the debate - will we benefit from selecting when the bots run? |
|
Hello, this is the Office Hour Bot. This is a reminder that the Hiero Python SDK Office Hours are scheduled in approximately 4 hours (14:00 UTC). This session provides an opportunity to ask questions regarding this Pull Request or receive assistance from a maintainer. Details:
Disclaimer: This is an automated reminder. Please verify the schedule here to be notified of any changes. |
|
Hi @tech0priyanshu, This pull request has had no commit activity for 10 days. Are you still working on the issue? please push a commit to keep the PR active or it will be closed due to inactivity. From the Python SDK Team |
|
temporarily closing this PR while it is parked |
Description:
Related issue(s):
Fixes #981
Checklist