Use espresso-types without testing feature #2283
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sveitser
commented
Nov 14, 2024
sveitser
added a commit
to EspressoSystems/hotshot-query-service
that referenced
this pull request
Nov 14, 2024
In the sequencer repo we don't have a proper `Default` implentation but one that's gated behind the testing feature because it wouldn't actually work. Working on EspressoSystems/espresso-sequencer#2283 it seems that this is one of the pain points for removing the spread of the testing features in the sequencer crates. It looks like we don't need the trait bound anymore in the query service.
We would like to be able to use the types crate in other crates but currently it won't compile without the testing feature enabled. This PR fixes that. - Use `NullStateCatchup` instead of `MockStateCatchup` where we don't need catchup. - Feature gate `MockStateCatchup` behind testing in types crate. - Remove the unused `InstanceState` argument from `from_transactions_sync`.
- `testing` feature enables `testing` feature of types and utils crate. - Running tests enables `testing` feature of crate via dev-dependencies. This allows to run `cargo check` and `cargo check --tests` in the sequencer crate. Previously `cargo check` was broken without the `testing` feature enabled explicitly.
sveitser
added a commit
to EspressoSystems/hotshot-query-service
that referenced
this pull request
Nov 15, 2024
* Remove unused Default bound on InstanceState In the sequencer repo we don't have a proper `Default` implentation but one that's gated behind the testing feature because it wouldn't actually work. Working on EspressoSystems/espresso-sequencer#2283 it seems that this is one of the pain points for removing the spread of the testing features in the sequencer crates. It looks like we don't need the trait bound anymore in the query service. * CI: run on all pull requests
sveitser
force-pushed
the
ma/types-create-without-testing-feature
branch
from
ff008fd to
9d3b50b
Compare
The libp2p feature was always turned on and the code didn't compile without it. With this change all feature sets checked with `cargo hack check --each-feature` compile successfully.
sveitser
requested review from
philippecamacho,
ImJeremyHe,
jbearer,
tbro and
imabdulbasit
as code owners
sveitser
commented
Nov 15, 2024
| @@ -488,15 +482,6 @@ pub async fn init_node<P: PersistenceOptions, V: Versions>( | |||
| )) | |||
| }; | |||
|
|
|||
| // Wait for the CDN network to be ready if we're not using the P2P network | |||
There was a problem hiding this comment.
@rob-maron I removed this wait and think that should be fine because we always have the libp2p enabled. Let me know if you see an issue with this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We would like to be able to use the types crate in other crates but currently it won't compile without the testing feature enabled. This PR fixes that.
NullStateCatchupinstead ofMockStateCatchupwhere we don't need catchup.MockStateCatchupbehind testing in types crate.InstanceStateargument fromfrom_transactions_sync.cargo hack.After this change I can add the
typescrate as a dependency in a rust project without turning on thetestingfeature and runcargo checksuccessfully.Closes #1491