Only automatically assign milestone on merge of PRs#18396
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refines the milestone assignment workflow so that only merged pull requests (not all closed issues) receive an automatic milestone.
- Drop the
issues: closedtrigger and related issue-closure logic - Update scripts to reference only
context.payload.pull_requestand check formerged === true - Narrow permissions to only pull-request operations (note: may need to revisit issue permissions)
Comments suppressed due to low confidence (2)
.github/workflows/assign-milestone-on-close.yml:11
- The workflow still calls
github.rest.issues.updateto assign milestones but no longer includesissues: writepermission. Please re-addissues: writeso the action can update milestones successfully.
pull-requests: write
.github/workflows/assign-milestone-on-close.yml:31
- [nitpick] Consider renaming this step ID from
check-donetocheck-mergedto match the updated behavior and step name.
id: check-done
|
I disagree that this adds work. I think it's useful to track when issues get closed to milestones |
|
I thought we (the team, not just Sean and I) discussed this previously, and decided to remove the issue milestoning part. What value does automatically adding issues closed without an associated PR to a milestone have? |
|
I might have been absent from this discussion. Quite often issues aren't linked correctly or are closed in relation to work in external repositories such as on our server. What harm does adding milestones to issues closed indirectly during a milestone period do? The only problem I can think of is when issues are reopened they are still left on the milestone, but that should be handled when someone reopens an issue, which is a manual task anyway |
|
Personally, I think this is more reasonable, as it clearly indicates the nature of the issue. A possible use case is when there is a closed issue that was not resolved by NVDA's PR, but rather by an external fix or workaround. However, the milestone may mislead users into thinking that it was resolved in a certain version of NVDA. Although this example may be extreme, and I don't always pay attention to milestone information, it makes sense. |
|
@seanbudd having unrelated (irrelevant or externally fixed) issues and PRs on the milestone:
|
|
If externally fixed issues are not put anymore in the milestone, a "fixed-externally" label could be used? Also, there is the case when the issue is fixed because not reproducible, but it has not been investigated what was the reason of the fix (change in NVDA or externally or something else) |
seanbudd
left a comment
There was a problem hiding this comment.
@SaschaCowley - thanks for explaining
Summary of the issue: The auto milestone action has been broken since #18396. Description of user facing changes: None Description of developer facing changes: Pull requests should have a milestone automatically applied on merge. Description of development approach: Gave the action write permission for issues. This is needed because PRs are considered to be a type of issue by GitHub. The action still needs write permission for pull requests as only users with merge permissions can add a milestone to pull requests.
Summary of the issue: The auto milestone action has been broken since nvaccess#18396. Description of user facing changes: None Description of developer facing changes: Pull requests should have a milestone automatically applied on merge. Description of development approach: Gave the action write permission for issues. This is needed because PRs are considered to be a type of issue by GitHub. The action still needs write permission for pull requests as only users with merge permissions can add a milestone to pull requests.
Update the documentation for the release process: - We no longer automatically add issues to the milestone since #18396 - add info on manually adding milestones - add a more useful explanation of NVDA release timings
Link to issue number:
None
Summary of the issue:
NV Access has a workflow set up to automatically apply a milestone to closed issues and merged pull requests. While this script only applies a milestone to issues closed as completed, many issues get closed as completed even when not planned or duplicates. This results in unnecessary work for staff.
Description of user facing changes:
None
Description of developer facing changes:
Only merged PRs are automatically given a milestone.
This matches our approach of applying milestones manually to issues planned for inclusion in particular releases.
Description of development approach:
Updated
.github/workflows/apply-milestone-on-close.ymlto remove the code for applying milestones to issues.Testing strategy:
Known issues with pull request:
It would be nice if we recorded the alpha, beta, and stable/rc milestones, and applied the correct milestone based on the branch into which the PR was merged.
Code Review Checklist:
@coderabbitai summary