Skip to content

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions GitForWindowsHelper/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The 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 main. That would cut out one extra round trip between GitHub and the GitForWindowsHelper, and simplify the mental model of the process.

Copy link
Member Author

Choose a reason for hiding this comment

The 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 release.released approach would get that for free.

Copy link
Member

Choose a reason for hiding this comment

The 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 closeReleaseMilestone()?

} 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'
Expand Down
42 changes: 41 additions & 1 deletion GitForWindowsHelper/update-milestones.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,47 @@ const renameCurrentAndCreateNextMilestone = async (context, req) => {
await openNextReleaseMilestone(context, await getToken(), owner, repo)
}

const closeReleaseMilestone = async (context, req) => {
const gitVersionMatch = req.body.pull_request.title.match(/^Rebase to (v\d+\.\d+\.\d+)$/)
if (!gitVersionMatch) throw new Error(`Not a new Git version: ${req.body.pull_request.title}`)
const gitVersion = gitVersionMatch[1]

const owner = 'git-for-windows'
const repo = 'git'
const sender = req.body.sender.login

const getToken = (() => {
let token

const get = async () => {
const getInstallationIdForRepo = require('./get-installation-id-for-repo')
const installationId = await getInstallationIdForRepo(context, owner, repo)
const getInstallationAccessToken = require('./get-installation-access-token')
return await getInstallationAccessToken(context, installationId)
}

return async () => token || (token = await get())
})()

const isAllowed = async (login) => {
if (login === 'gitforwindowshelper[bot]') return true
const getCollaboratorPermissions = require('./get-collaborator-permissions')
const token = await getToken()
const permission = await getCollaboratorPermissions(context, token, owner, repo, login)
return ['ADMIN', 'MAINTAIN', 'WRITE'].includes(permission.toString())
}

if (!await isAllowed(sender)) throw new Error(`${sender} is not allowed to do that`)

const { getMilestoneByName, closeMilestone } = require('./milestones')
const current = await getMilestoneByName(console, await getToken(), owner, repo, gitVersion)
if (current.open_issues > 0) throw new Error(`Milestone ${current.title} has ${current.open_issues} open issue(s)!`)
if (current.closed_issues == 0) throw new Error(`Milestone ${current.title} has no closed issue(s)!`)
await closeMilestone(context, await getToken(), owner, repo,current.id)
}

module.exports = {
addIssueToCurrentMilestone,
renameCurrentAndCreateNextMilestone
renameCurrentAndCreateNextMilestone,
closeReleaseMilestone
}