payjoin
Introduce Payjoin version enum
Pull Request Test Coverage Report for Build 14868573150
Details
- 20 of 28 (71.43%) changed or added relevant lines in 6 files are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage decreased (-0.03%) to 81.995%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| payjoin/src/core/version.rs | 0 | 8 | 0.0% |
| <!-- | Total: | 20 | 28 |
| Totals | |
|---|---|
| Change from base Build 14867604163: | -0.03% |
| Covered Lines: | 5442 |
| Relevant Lines: | 6637 |
💛 - Coveralls
have you considered an explicit discriminant enum? I can't imagine our Version enum would ever have fields unless we did something wonky to have sub-versions.
Something like the following is what I was thinking about. I'm not suggesting a change per se but trying to understand the rationale for one over the other if there was one
#[repr(u8)]
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Version {
One = 1,
Two = 2,
}
@DanGould I hadn't considered it, but I think it makes more sense to have it as an explicit discriminant enum so we don't need the extra logic to convert to a usize. I'm not too familiar with the #[repr(u8)] bit but did some reading, and it looks like it'll be necessary since it might get included in the ffi work