Skip to content
This repository has been archived by the owner on Mar 23, 2021. It is now read-only.

Implement Arbitrary directly on types where possible #3350

Merged
merged 1 commit into from Nov 4, 2020
Merged

Conversation

thomaseizinger
Copy link
Contributor

With the introduction swap execution in the comit lib, we also
introduced a dependency on quickcheck. Previously, a lot of the
Arbitrary impls where defined within nectar. We can now move
them to the comit lib.

@@ -15,6 +15,8 @@
#![type_length_limit = "1049479"] // Regressed with Rust 1.46.0 :(

pub mod actions;
#[cfg(feature = "quickcheck")]
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is test code I would expect it to be behind cfg(test)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it is behind cfg(test), you cannot use it from a different crate. It is quite common for library crates to expose such implementations under the feature-flag of the library that enables them. It is similar to how implementations of serde::Serialize are exposed under a serde feature flag.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it is behind cfg(test), you cannot use it from a different crate.

Can you clarify how it is behind cfg(test)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It isn't. If it were, then all consumers of the comit crate would not be able to use the functions in this module. This is the comit crate in case you didn't notice :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you saying that we don't need to expose this module publicly because neither cnd nor nectar are currently using the functions in this module?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, thinking about it. It's fine the way it is :)

With the introduction swap execution in the comit lib, we also
introduced a dependency on quickcheck. Previously, a lot of the
`Arbitrary` impls where defined within nectar. We can now move
them to the `comit` lib.
Base automatically changed from resilient-swap-execution to dev November 4, 2020 03:12
@mergify
Copy link
Contributor

mergify bot commented Nov 4, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 4, 2020

@bors bors bot merged commit 64d09cd into dev Nov 4, 2020
@bors bors bot deleted the better-arbitrary branch November 4, 2020 03:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants