Skip to content
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

27.1.0 #755

Merged
merged 8 commits into from
Mar 28, 2022
Merged

27.1.0 #755

merged 8 commits into from
Mar 28, 2022

Conversation

github-actions[bot]
Copy link
Contributor

This is the release candidate for version 27.1.0.

@gantunesr gantunesr marked this pull request as ready for review March 25, 2022 16:13
@gantunesr gantunesr requested a review from a team as a code owner March 25, 2022 16:13
adonesky1
adonesky1 previously approved these changes Mar 25, 2022
Copy link
Contributor

@adonesky1 adonesky1 left a comment

Choose a reason for hiding this comment

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

LGTM!

mcmire
mcmire previously approved these changes Mar 25, 2022
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

LGTM too.

Co-authored-by: Mark Stacey <[email protected]>
Co-authored-by: Mark Stacey <[email protected]>
@gantunesr gantunesr requested review from mcmire and Gudahtt March 26, 2022 04:04
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@adonesky1 adonesky1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gantunesr gantunesr merged commit dc151ad into main Mar 28, 2022
@gantunesr gantunesr deleted the release/27.1.0 branch March 28, 2022 14:20
@wachunei
Copy link
Member

TypeScript consumer of this version (like swaps-controller) will fail on type checking because KeyringController imports the types from @keystonehq/metamask-airgapped-keyring

https://github.com/MetaMask/controllers/blob/main/src/keyring/KeyringController.ts#L17-L20

But that dependency is declared in devDependencies only, so they are not installed in controllers consumers.

Screen Shot 2022-03-30 at 09 37 17

~/p/c/controllers ❯❯❯ yarn why @keystonehq/metamask-airgapped-keyring                                                                                                                                                                 main
yarn why v1.22.17
[1/4] 🤔  Why do we have the module "@keystonehq/metamask-airgapped-keyring"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "@keystonehq/[email protected]"
info Has been hoisted to "@keystonehq/metamask-airgapped-keyring"
info This module exists because it's specified in "devDependencies".

@mcmire
Copy link
Contributor

mcmire commented Mar 30, 2022

Ugh I missed that on the PR 😩 Okay we should probably issue a hotfix moving this to dependencies.

@wachunei
Copy link
Member

@mcmire will import type be enough?
Yeah we should deprecate this release 😢

@mcmire
Copy link
Contributor

mcmire commented Mar 30, 2022

PR to address this here: #757

@mcmire
Copy link
Contributor

mcmire commented Mar 30, 2022

@wachunei No but I think if you add @keystonehq/metamask-airgapped-keyring to dependencies in swaps-controller this may solve the issue temporarily.

MajorLift pushed a commit that referenced this pull request Oct 11, 2023
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
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.

5 participants