Skip to content

Conversation

@cho-m
Copy link
Member

@cho-m cho-m commented Nov 23, 2025

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?

Seeing if possible to remove deprecated cask via test-bot to unblock CI. May need to also either discuss deprecation with GitHub side, and/or see if it can be migrated elsewhere.

There is an issue for aws tap to provide their own session-manager-plugin and another issue in upstream to sign/notarize binary. Depending on license, could check if possible to make a Formula.

Also trying to see if HOMEBREW_CASKROOM from startup/config.rb works rather than needing to use Cask::Caskroom.path (which adds an extra require)

Copilot AI review requested due to automatic review settings November 23, 2025 20:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to simplify the codebase by using the HOMEBREW_CASKROOM constant (defined in startup/config.rb) instead of requiring and using Cask::Caskroom.path. The change adds a cleanup operation to remove the session-manager-plugin cask during test bot cleanup on macOS.

Key Changes

  • Adds cleanup of session-manager-plugin from the caskroom during test bot cleanup before tests run on macOS
  • Leverages the existing HOMEBREW_CASKROOM constant to avoid an extra require statement

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@cho-m cho-m force-pushed the session-manager-plugin-remove branch from 758b61c to d279703 Compare November 23, 2025 20:07
@cho-m cho-m marked this pull request as ready for review November 23, 2025 20:14
Copilot AI review requested due to automatic review settings November 23, 2025 20:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks again @cho-m!

delete_or_move bad_paths, sudo: true
elsif OS.mac?
delete_or_move HOMEBREW_CELLAR.glob("*")
delete_or_move HOMEBREW_CASKROOM.glob("session-manager-plugin")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
delete_or_move HOMEBREW_CASKROOM.glob("session-manager-plugin")
delete_or_move HOMEBREW_CASKROOM.glob("*")

we should consider this as a future iteration

@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Nov 24, 2025
Merged via the queue into main with commit bc24254 Nov 24, 2025
41 checks passed
@MikeMcQuaid MikeMcQuaid deleted the session-manager-plugin-remove branch November 24, 2025 09:45
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.

3 participants