Finish milestone by jennyhickson · Pull Request #178 · MetOffice/SimSys_Scripts

Jenny Hickson (jennyhickson) · 2026-02-03T18:17:42Z

PR Summary

Sci/Tech Reviewer:
Code Reviewer: James Bruten (@james-bruten-mo)

New script for closing a milestone from the Review Tracker project during a release. It:

checks if there are still open PRs against the milestone
checks if there are closed PRs against another milestone, and changes closed PRs with no milestone to the current one
reports on the number of PRs closed for the milestone
archives all closed PRs for the milestone
gives the command for closing the milestone in each repo

To do this it also improves the project data class, simplifying the data structure, including functions for handling milestones and storing the PR details in objects rather than a dictionary.

The user will need to authenticate themselves to modify the project, github prompts for this on first use.

Code Quality Checklist

I have performed a self-review of my own code
My code follows the project's style guidelines
Comments have been included that aid understanding and enhance the readability of the code
My changes generate no new warnings
All automated checks in the CI pipeline have completed successfully

Testing

This change has been tested appropriately (please describe)

Security Considerations

I have reviewed my changes for potential security issues
Sensitive data is properly handled (if applicable)
Authentication and authorisation are properly implemented (if applicable)

AI Assistance and Attribution

Some of the content of this change has been produced with the assistance of Generative AI tool name (e.g., Met Office Github Copilot Enterprise, Github Copilot Personal, ChatGPT GPT-4, etc) and I have followed the Simulation Systems AI policy (including attribution labels)

Sci/Tech Review

I understand this area of code and the changes being added
The proposed changes correspond to the pull request description
Documentation is sufficient (do documentation papers need updating)
Sufficient testing has been completed

(Please alert the code reviewer via a tag when you have approved the SR)

Code Review

All dependencies have been resolved
Related Issues have been properly linked and addressed
Code quality standards have been met
Tests are adequate and have passed
Security considerations have been addressed
Performance impact is acceptable

# Conflicts: # gh_review_project/review_project.py # gh_review_project/test/test.json

James Bruten (james-bruten-mo)

Just a couple of suggestions

James Bruten (james-bruten-mo) · 2026-02-04T14:22:13Z

 import shlex
 from collections import defaultdict

+project_id = 376


Suggested change

project_id = 376

PROJECT_ID = 376

James Bruten (james-bruten-mo) · 2026-02-04T14:22:25Z

 from collections import defaultdict

+project_id = 376
+project_owner = "MetOffice"


Suggested change

project_owner = "MetOffice"

PROJECT_OWNER = "MetOffice"

James Bruten (james-bruten-mo) · 2026-02-04T14:23:36Z

-            raise RuntimeError(
-                "Error fetching GitHub Project data:  \n " + output.stderr.decode()
-            )
+        command = f"gh project item-list {project_id} -L 500 --owner {project_owner} --format json"


It feels like project_id/owner should be inputs to this function, rather than hard-coded to the global variable.
Maybe def from_github(cls, project_id=PROJECT_ID, project_owner=PROJECT_OWNER...)?

given there is much in this code that is specific to this project, I've left it just pulling directly from the globals for now since there are bigger changes needed to make this an option.

James Bruten (james-bruten-mo) · 2026-02-04T14:34:59Z

        """

-        data = defaultdict(list)
+        data = []


Would pull_requests or pull_requests_data be a clearer name?

I guess same with self.data? If it's not just pr's then fair enough

James Bruten (james-bruten-mo) · 2026-02-04T14:50:31Z

+        dry_run: If true, print the command used rather than archiving.
+        """
+
+        command = f"gh project item-archive {project_id} --owner {project_owner} --id {self.id}"


Similar comments around project_owner/id as above

James Bruten (james-bruten-mo) · 2026-02-04T14:50:40Z

+                pr.archive(dry_run)
+
+
+class PullRequest:


Probably wants a docstring

yep. Hopefully doc strings everywhere now?

James Bruten (james-bruten-mo) · 2026-02-04T14:50:59Z

+        self.scitechReview = None
+        self.codeReview = None
+
+    def archive(self, dry_run: bool = False):


Suggested change

def archive(self, dry_run: bool = False):

def archive(self, dry_run: bool = False) -> None:

James Bruten (james-bruten-mo) · 2026-02-04T15:11:47Z

+    exceptions.
+    """
+    total_open = still_open(open_prs[milestone], milestone)
+    total_other = closed_other(closed_prs, milestone)


Do we want to update the milestone on these PRs to match the closing milestone? I guess you'd logically do it after the check_ready step and before report?

I didn't think I could, but worked it out now. Added to the closed_other function as no harm in doing this even if you're not ready to fully archive things.

This also had the side effect of reading things directly from the data everywhere as the modify_milestone function changes things so the copies I'd taken of open and closed PRs by milestone were then out of date.

James Bruten (james-bruten-mo) · 2026-02-04T15:23:50Z

+        * Remaining open PRs and issues against this milestone
+        * Closed PRs against this milestone
+"""
+from collections import defaultdict


Not sure you're using this in this file?

James Bruten (james-bruten-mo)

Thanks Jenny, changes look good

Jenny Hickson (jennyhickson) added 21 commits January 29, 2026 12:35

Expand list of repositories for milestones

beba078

Extract project class into own file

eaef11d

Rename directory

5bb7738

tidy unneeded structure.

5b0bd95

typo

5210898

black

e11cfa5

refactor with classmethods

6c8214a

black

134dc39

process filepath better

94a9e0f

add milestone script

c756e7a

CR suggestions

0c2ab39

functional

de0bc74

make data a list

8aee595

tidy milestone getter

940973d

update captured data

cd34f3b

Merge branch 'refactor_workload' into finish_milestone

7b43072

add total

f57c618

Merge remote-tracking branch 'upstream/main' into finish_milestone

f6e3fc5

# Conflicts: # gh_review_project/review_project.py # gh_review_project/test/test.json

get milestones and repos

85a0fe4

tidy type hinting

67d21d2

black

5e14642

github-actions Bot assigned Jenny Hickson (jennyhickson) Feb 3, 2026

Jenny Hickson (jennyhickson) requested a review from James Bruten (james-bruten-mo) February 4, 2026 12:24

James Bruten (james-bruten-mo) reviewed Feb 4, 2026

View reviewed changes

change milestones for closed PRs

3b95d2f

Jenny Hickson (jennyhickson) requested a review from James Bruten (james-bruten-mo) February 10, 2026 14:15

black

65bcb9f

James Bruten (james-bruten-mo) approved these changes Feb 10, 2026

View reviewed changes

Merge branch 'main' into finish_milestone

2e09c1b

James Bruten (james-bruten-mo) merged commit 7fd9c30 into MetOffice:main Feb 10, 2026
5 checks passed

Jenny Hickson (jennyhickson) added this to the Spring 2026 milestone Feb 11, 2026

	def archive(self, dry_run: bool = False):
	def archive(self, dry_run: bool = False) -> None:

Conversation

Jenny Hickson (jennyhickson) commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Code Quality Checklist

Testing

Security Considerations

AI Assistance and Attribution

Sci/Tech Review

Code Review

Uh oh!

James Bruten (james-bruten-mo) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

James Bruten (james-bruten-mo) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Jenny Hickson (jennyhickson) commented Feb 3, 2026 •

edited

Loading