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

fix(cli): Update warp apply to apply changes in single command #4672

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ltyu
Copy link
Contributor

@ltyu ltyu commented Oct 11, 2024

Description

This PR fixes a limitation in warp apply such that it can only extend or update an existing warp route. This means that for configs with both changes require warp apply to be called multiple times. An example is when Renzo deploys to new chain, and it needs to update the existing ISMs.

Related issues

Backward compatibility

Yes

Testing

Manual/Unit Tests

Copy link

changeset-bot bot commented Oct 11, 2024

🦋 Changeset detected

Latest commit: e4be6d8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@hyperlane-xyz/cli Minor
@hyperlane-xyz/ccip-server Minor
@hyperlane-xyz/github-proxy Minor
@hyperlane-xyz/helloworld Minor
@hyperlane-xyz/infra Minor
@hyperlane-xyz/sdk Minor
@hyperlane-xyz/utils Minor
@hyperlane-xyz/widgets Minor
@hyperlane-xyz/core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ltyu ltyu requested a review from paulbalaji October 11, 2024 17:01
Copy link
Member

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

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

mostly lgtm although it would intuitively make more sense to me if we did

contracts = contracts.extend()
contracts.update()

Comment on lines +509 to +514
const mergedRouters = mergeAllRouters(
multiProvider,
existingConfigs,
newDeployedContracts,
warpCoreConfigByChain,
);
Copy link
Member

Choose a reason for hiding this comment

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

can we just join the newDeployedContracts with the existing deployed and pass that to the update step?

Copy link

codecov bot commented Oct 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.85%. Comparing base (adedb3d) to head (e4be6d8).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4672   +/-   ##
=======================================
  Coverage   73.85%   73.85%           
=======================================
  Files         100      100           
  Lines        1423     1423           
  Branches      181      181           
=======================================
  Hits         1051     1051           
  Misses        351      351           
  Partials       21       21           
Components Coverage Δ
core 84.61% <ø> (ø)
hooks 75.71% <ø> (ø)
isms 78.94% <ø> (ø)
token 88.23% <ø> (ø)
middlewares 77.39% <ø> (ø)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

fix: warp apply requires multiple calls for multiple updates
3 participants