Skip to content

Only automatically assign milestone on merge of PRs #18396

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SaschaCowley
Copy link
Member

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.yml to 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:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@coderabbitai summary

@Copilot Copilot AI review requested due to automatic review settings July 2, 2025 03:26
@SaschaCowley SaschaCowley requested a review from a team as a code owner July 2, 2025 03:26
@SaschaCowley SaschaCowley requested a review from seanbudd July 2, 2025 03:26
Copy link

@Copilot Copilot AI left a 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 refines the milestone assignment workflow so that only merged pull requests (not all closed issues) receive an automatic milestone.

  • Drop the issues: closed trigger and related issue-closure logic
  • Update scripts to reference only context.payload.pull_request and check for merged === 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.update to assign milestones but no longer includes issues: write permission. Please re-add issues: write so 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-done to check-merged to match the updated behavior and step name.
        id: check-done

@seanbudd
Copy link
Member

seanbudd commented Jul 2, 2025

I disagree that this adds work. I think it's useful to track when issues get closed to milestones

@seanbudd seanbudd added the blocked/needs-product-decision A product decision needs to be made. Decisions about NVDA UX or supported use-cases. label Jul 2, 2025
@SaschaCowley
Copy link
Member Author

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?
For instance, the following issues are all currently (2025-07-02, ~14:00 UTC+10:00) on the 2025.3 milestone, despite their closure having nothing to do with 2025.3: #6640, #6750, #6891, #7250, #7540, #7550, #11345, #17890, #18388, and (arguably) #18347.

@seanbudd
Copy link
Member

seanbudd commented Jul 2, 2025

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

@hwf1324
Copy link
Contributor

hwf1324 commented Jul 2, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked/needs-product-decision A product decision needs to be made. Decisions about NVDA UX or supported use-cases.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants