Skip to content

Conversation

@Kayanski
Copy link
Contributor

@Kayanski Kayanski commented Apr 8, 2024

This Pr aims at loosening some trait bounds in cw-orch-core because theyr are un-necessary

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Apr 8, 2024

Deploying cw-orchestrator with  Cloudflare Pages  Cloudflare Pages

Latest commit: be82090
Status: ✅  Deploy successful!
Preview URL: https://d86b11d3.cw-orchestrator.pages.dev
Branch Preview URL: https://nicolas-orc-104-trait-bounds.cw-orchestrator.pages.dev

View logs

@codecov
Copy link

codecov bot commented Apr 8, 2024

Codecov Report

Attention: Patch coverage is 91.66667% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 64.7%. Comparing base (ff1e75d) to head (be82090).
Report is 3 commits behind head on main.

Additional details and impacted files
Files Coverage Δ
contracts/counter/src/interface.rs 42.1% <ø> (ø)
contracts/mock_contract/src/lib.rs 97.9% <ø> (ø)
contracts/mock_contract/src/msg_tests.rs 94.7% <ø> (ø)
contracts/mock_contract_u64/src/lib.rs 83.3% <ø> (ø)
cw-orch-daemon/src/channel.rs 82.9% <100.0%> (ø)
cw-orch/src/snapshots.rs 97.5% <ø> (ø)
packages/cw-orch-contract-derive/src/lib.rs 93.0% <100.0%> (ø)
packages/cw-orch-core/src/contract/deploy.rs 0.0% <ø> (ø)
...ages/cw-orch-core/src/contract/interface_traits.rs 75.9% <ø> (ø)
packages/cw-orch-fns-derive/src/execute_fns.rs 99.2% <100.0%> (ø)
... and 2 more

@CyberHoward CyberHoward changed the title Loosen som trait bounds in cw-orch-core Loosen trait bounds in cw-orch-core Apr 9, 2024
}

/// Helper methods for conditional uploading of a contract.
pub trait ConditionalUpload<Chain: CwEnv>: CwOrchUpload<Chain> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we loosen this bound as well? Could you search for all the uses of CwEnv in this library and replace them with their minimal requirements?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I did yes ! And we can't loosen that bound because you need both QueryHandler and TxHandler to be able to use this trait !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

emoved in the proc macros as well :)

Copy link
Contributor

@CyberHoward CyberHoward left a comment

Choose a reason for hiding this comment

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

amazing, is this a breaking change to core?

@Kayanski
Copy link
Contributor Author

Kayanski commented Apr 9, 2024

amazing, is this a breaking change to core?

I'm not sure how to test that :/
Because we don't have abstract on that version, so it won't be all covering

@Kayanski
Copy link
Contributor Author

Kayanski commented Apr 9, 2024

amazing, is this a breaking change to core?

It shouldn't be, but I'm not sure there were not changes in the main branch that were breaking

@Kayanski Kayanski merged commit 9145303 into main Apr 9, 2024
@Kayanski Kayanski deleted the nicolas/orc-104-trait-bounds-too-strict branch April 9, 2024 17:07
@CyberHoward
Copy link
Contributor

amazing, is this a breaking change to core?

It shouldn't be, but I'm not sure there were not changes in the main branch that were breaking

This is important for you to keep track of. You can go through commits on main and see if any of them are PRs that you expect to be breaking and look into them.

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