-
Notifications
You must be signed in to change notification settings - Fork 6
WIP: Address various issues about milestones #143
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
a27115e
adc0fb5
b6e6295
3e20881
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,6 +83,17 @@ module.exports = async function (context, req) { | |
| return withStatus(500, undefined, e.message || JSON.stringify(e, null, 2)) | ||
| } | ||
|
|
||
| try { | ||
| const {closeReleaseMilestone} = require('./update-milestones') | ||
| if (req.headers['x-github-event'] === 'pull_request' | ||
| && req.body.repository.full_name === 'git-for-windows/git' | ||
| && req.body.action === 'closed' | ||
| && req.body.pull_request.merged === 'true') return ok(await closeReleaseMilestone(context, req)) | ||
|
Comment on lines
+88
to
+91
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternatively, we could piggy-back onto the code that "pushes" the PR branch to
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does cut out a roundtrip, but it does require duplication of the "is this a pre-release" logic from https://github.com/git-for-windows/git-for-windows-automation/blob/main/.github/actions/github-release/action.yml and maybe some special handling of out of band releases, whereas the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I missed that. But wouldn't we want much more narrow conditions to be met before calling |
||
| } catch (e) { | ||
| context.log(e) | ||
| return withStatus(500, undefined, e.message || JSON.stringify(e, null, 2)) | ||
| } | ||
|
|
||
| try { | ||
| const { cascadingRuns, handlePush } = require('./cascading-runs.js') | ||
| if (req.headers['x-github-event'] === 'check_run' | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather do that when publishing the final release, i.e. at the same time as the current milestone is closed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would prevent issues with late component updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it the other way round? When a final rebase PR is opened, and
/git-artifactsproduces an installer that has issues that require acomponent-updateticket to be addressed, we'd still want that in the same milestone, right?