Skip to content

Replace release bot#63

Merged
MahdiBM merged 35 commits intomainfrom
mmbm-replace-release-bot
Jul 10, 2023
Merged

Replace release bot#63
MahdiBM merged 35 commits intomainfrom
mmbm-replace-release-bot

Conversation

@MahdiBM
Copy link
Copy Markdown
Collaborator

@MahdiBM MahdiBM commented Jul 4, 2023

Check list

  • Catch PR closed events where merged is true and detect the semver-patch/minor/major labels.
  • Create new releases.
    • Determine the latest release through the API. Look for the latest tag. Don't just do releases.last.
  • Add comments on PRs.
  • Add auth headers and stuff to Github requests.
  • Send message to Discord.
  • Add GH-related secrets.
  • /latest-release doesn't give prereleases?
  • Before deploy, consider it'll duplicate everything the release bot does (releases, comments etc..).
  • Open up the release channel for Penny to send messages to.
    • Don't do it before merge or there will be the problem below:
  • Make sure Penny doesn't send thanks-responses to the release channel since it'll have send-messages perm.
  • Tests

@MahdiBM MahdiBM marked this pull request as draft July 4, 2023 23:11
@MahdiBM MahdiBM requested a review from gwynne July 5, 2023 03:22
Copy link
Copy Markdown
Member

@gwynne gwynne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per inline comments. You're gonna hate me for this one.

Comment thread Lambdas/GHHooks/Constants.swift
Comment thread Lambdas/GHHooks/EventHandler/EventHandler.swift
Comment thread Lambdas/GHHooks/EventHandler/Handlers/PRHandler.swift Outdated
Comment thread Lambdas/GHHooks/Models/+PR.Label.swift
Comment thread Lambdas/GHHooks/Models/SemVer.swift Outdated
@MahdiBM MahdiBM requested a review from gwynne July 5, 2023 06:16
@MahdiBM MahdiBM marked this pull request as ready for review July 5, 2023 11:30
@MahdiBM MahdiBM changed the title Replace release-bot Replace release bot Jul 5, 2023
@MahdiBM MahdiBM marked this pull request as draft July 5, 2023 11:34
Copy link
Copy Markdown
Member

@gwynne gwynne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, LGTM so far 🙂

@MahdiBM MahdiBM marked this pull request as ready for review July 5, 2023 19:22
@MahdiBM MahdiBM requested a review from gwynne July 5, 2023 22:09
@MahdiBM
Copy link
Copy Markdown
Collaborator Author

MahdiBM commented Jul 6, 2023

There is a known warning in GHLazyMiddleware which will be solved by apple/swift-openapi-runtime#22.

Copy link
Copy Markdown
Member

@gwynne gwynne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of these are tiny nits, others are serious concerns. At least two are actual logic errors, and there's a major threading issue to boot. (At least, assuming I didn't misread anything or do the logic in my head wrong, we all know that happens a lot 😆)

Comment thread Lambdas/GHHooks/EventHandler/EventHandler.swift
Comment thread Lambdas/GHHooks/EventHandler/Handlers/PRHandler.swift Outdated
Comment thread Lambdas/GHHooks/EventHandler/Handlers/PRHandler.swift Outdated
Comment thread Lambdas/GHHooks/GHLazyMiddleware.swift Outdated
Comment thread Lambdas/GHHooks/Models/+SemanticVersion.swift Outdated
Comment thread Sources/Penny/+String.swift
Comment thread Tests/Fake/TestData.swift Outdated
Comment thread Tests/PennyTests/GHHooksTests.swift Outdated
Comment thread deploy/penny-discord-bot-stack.yml Outdated
Comment thread Tests/PennyTests/GHHooksTests.swift
@gwynne
Copy link
Copy Markdown
Member

gwynne commented Jul 9, 2023

@MahdiBM Watch out for Github's tendency to hide comments behind "5 hidden conversations" etc. links on the Conversation page of PRs; I miss comments that way more often than I care to admit 🤦‍♀️.

@MahdiBM MahdiBM requested a review from gwynne July 10, 2023 04:28
Co-authored-by: Gwynne Raskind <gwynne@vapor.codes>
Copy link
Copy Markdown
Member

@gwynne gwynne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there, I think 😰 Sorry that I ended up worrying about threading stuff anyway after all that - I'm considerably less confident of my conclusions this time, but it nonetheless seems correct to me after careful reading of the linked info. Please do feel free to challenge my reasoning if you see any (more) mistakes 😅

Comment thread Lambdas/GHHooks/GHLazyMiddleware.swift Outdated
Comment thread Lambdas/GHHooks/SecretsRetriever.swift Outdated
Comment thread Lambdas/GHHooks/SecretsRetriever.swift Outdated
Comment thread Lambdas/GHHooks/SecretsRetriever.swift Outdated
Comment thread Tests/PennyTests/GHHooksTests.swift
@MahdiBM MahdiBM requested a review from gwynne July 10, 2023 05:38
Copy link
Copy Markdown
Member

@gwynne gwynne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, finally LGTM 😅 Excellent work!

@MahdiBM MahdiBM enabled auto-merge (squash) July 10, 2023 07:32
@MahdiBM MahdiBM merged commit 1caac06 into main Jul 10, 2023
@MahdiBM MahdiBM deleted the mmbm-replace-release-bot branch July 10, 2023 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants