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

chore: two steps CI CD deployment #434

Merged
merged 6 commits into from
Nov 25, 2024
Merged

chore: two steps CI CD deployment #434

merged 6 commits into from
Nov 25, 2024

Conversation

khanti42
Copy link
Collaborator

Why This Change?

Goal

To safely test new snap versions in production (e.g. on MetaMask not MetaMask Flask) before forcing updates on end-users:

The new workflow would be

  1. Deploy new Snap to NPM
  2. Allowlist the new version
  3. Test the new version with MetaMask locally
  4. Deploy new wallet-ui and get-starknet

Solution

Split the workflow into two parts:

  1. Publish NPM:

    • Build and publish starknet-snap.
    • Output version for allowlisting and validation.
  2. Deploy UI and Get Starknet:

    • Build and deploy wallet-ui and get-starknet separately after snap validation.

Known Limitations

  • get-starknet Wildcard Behavior:
    • Always targets the latest allowlisted snap (*).
    • Cached Webpack modules in dApps may not fetch the updated snap, potentially causing errors.
  • Manual Steps:
    • This split workflow introduces a manual step between publishing the snap and deploying related components. However, this is necessary for controlled testing.

Benefits

  • Controlled production testing for snaps.
  • Prevents breaking changes for get-starknet users.
  • Flexible foundation for future deployment improvements.

@khanti42 khanti42 requested a review from a team as a code owner November 24, 2024 21:53
@khanti42 khanti42 requested review from Julink-eth and stanleyyconsensys and removed request for a team November 24, 2024 21:53
Copy link
Collaborator

@stanleyyconsensys stanleyyconsensys left a comment

Choose a reason for hiding this comment

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

some ENV setup incorrect
and i would suggest keep this 2 new pipeline for production only

.github/workflows/deploy-wallet-get-starknet.yml Outdated Show resolved Hide resolved
.github/workflows/deploy-wallet-get-starknet.yml Outdated Show resolved Hide resolved
.github/workflows/deploy-wallet-get-starknet.yml Outdated Show resolved Hide resolved
.github/workflows/deploy-wallet-get-starknet.yml Outdated Show resolved Hide resolved
.github/workflows/publish-npm.yml Outdated Show resolved Hide resolved
.github/workflows/publish-npm.yml Outdated Show resolved Hide resolved
.github/workflows/publish-npm.yml Show resolved Hide resolved
.github/workflows/publish-npm.yml Show resolved Hide resolved
Copy link
Collaborator

@stanleyyconsensys stanleyyconsensys left a comment

Choose a reason for hiding this comment

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

some remove of cache key, but not sure does it work, needed to test

.github/workflows/deploy-wallet-get-starknet.yml Outdated Show resolved Hide resolved
.github/workflows/deploy-wallet-get-starknet.yml Outdated Show resolved Hide resolved
.github/workflows/publish-npm.yml Outdated Show resolved Hide resolved
@khanti42 khanti42 changed the title chore: two steps CI/CD deployment chore: two steps CI CD deployment Nov 25, 2024
@khanti42 khanti42 enabled auto-merge November 25, 2024 15:06
Copy link

sonarcloud bot commented Nov 25, 2024

Quality Gate Passed Quality Gate passed for 'consensys_starknet-snap-wallet-ui'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copy link

sonarcloud bot commented Nov 25, 2024

Quality Gate Passed Quality Gate passed for 'consensys_starknet-snap-starknet-snap'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@khanti42 khanti42 added this pull request to the merge queue Nov 25, 2024
Merged via the queue into main with commit 6a0236b Nov 25, 2024
13 checks passed
@khanti42 khanti42 deleted the chore/sf-697-split-ci-cd branch November 25, 2024 15:22
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