Conversation
# Conflicts: # gh_review_project/review_project.py # gh_review_project/test/test.json
james-bruten-mo
left a comment
james-bruten-mo
left a comment
There was a problem hiding this comment.
Just a couple of suggestions
| import shlex | ||
| from collections import defaultdict | ||
|
|
||
| project_id = 376 |
There was a problem hiding this comment.
| project_id = 376 | |
| PROJECT_ID = 376 |
| from collections import defaultdict | ||
|
|
||
| project_id = 376 | ||
| project_owner = "MetOffice" |
There was a problem hiding this comment.
| project_owner = "MetOffice" | |
| PROJECT_OWNER = "MetOffice" |
| 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" |
There was a problem hiding this comment.
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...)?
| """ | ||
|
|
||
| data = defaultdict(list) | ||
| data = [] |
There was a problem hiding this comment.
Would pull_requests or pull_requests_data be a clearer name?
There was a problem hiding this comment.
I guess same with self.data? If it's not just pr's then fair enough
| 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}" |
There was a problem hiding this comment.
Similar comments around project_owner/id as above
| pr.archive(dry_run) | ||
|
|
||
|
|
||
| class PullRequest: |
There was a problem hiding this comment.
Probably wants a docstring
| self.scitechReview = None | ||
| self.codeReview = None | ||
|
|
||
| def archive(self, dry_run: bool = False): |
There was a problem hiding this comment.
| def archive(self, dry_run: bool = False): | |
| def archive(self, dry_run: bool = False) -> None: |
| exceptions. | ||
| """ | ||
| total_open = still_open(open_prs[milestone], milestone) | ||
| total_other = closed_other(closed_prs, milestone) |
There was a problem hiding this comment.
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?
| * Remaining open PRs and issues against this milestone | ||
| * Closed PRs against this milestone | ||
| """ | ||
| from collections import defaultdict |
There was a problem hiding this comment.
Not sure you're using this in this file?
2 participants
PR Summary
Sci/Tech Reviewer:
Code Reviewer: @james-bruten-mo
New script for closing a milestone from the Review Tracker project during a release. It:
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
Testing
Security Considerations
AI Assistance and Attribution
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review