-
Notifications
You must be signed in to change notification settings - Fork 19
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
Deps: Bump to core 0.5.1 #655
Conversation
To make tests works see: farcaster-project/farcaster-core#293 We have to create a [features]
nightly = ["farcaster_core/nightly"] Unit tests should then pass because of
So:
|
I am for 2. , the code coverage tool is not practical for our code base and I don't think I ever used it. |
Done in #656 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good, just one comment about type used in struct that requires strict encoding.
#[cfg_attr( | ||
feature = "serde", | ||
derive(Serialize, Deserialize), | ||
serde(crate = "serde_crate") | ||
)] | ||
#[display(TookOffer::to_yaml_string)] | ||
pub struct TookOffer { | ||
pub offerid: PublicOfferId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, as mentioned in farcaster-project/farcaster-core#292, we can use [u8; 16]
to avoid manual impl. of encoding. (But with manual From/Into calls in code when we want Uuid
types)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TookOffer is printed as a yaml string, so I want to preserve the Uuid type. I am not sure how a impl From
would help here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
No description provided.