Closed mindreader closed 4 years ago
Merging #102 into master will increase coverage by
0.03%
. The diff coverage is87.50%
.
@@ Coverage Diff @@
## master #102 +/- ##
==========================================
+ Coverage 79.58% 79.62% +0.03%
==========================================
Files 24 24
Lines 1739 1747 +8
==========================================
+ Hits 1384 1391 +7
- Misses 355 356 +1
Impacted Files | Coverage Δ | |
---|---|---|
src/extension/itunes/itunes_item_extension.rs | 85.18% <83.33%> (-0.15%) |
:arrow_down: |
tests/read.rs | 95.36% <100.00%> (+0.02%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update b945f5e...cfec67d. Read the comment docs.
@mindreader, according to specification:
Where episode is a non-zero integer (1, 2, 3, etc.) representing your episode number.
So it should be represented by integer
type(u16
?), not by String
@mindreader @AnderEnder Formally NonZeroU32
(or NonZeroU16
) should be used. But this extension has already many fields which could be represented with a more precise types, but are represented as String
(duration
and explicit
are good examples).
Personally I am ok with String
s.
I originally started writing this with number types, but noticed the rest of the library uses strings, especially for things like dates. I decided that is probably actually a good thing. Parse as leniently as possible and then have the validator suggest improvements.
On Sun, Aug 2, 2020, 08:39 Andrey Kutejko notifications@github.com wrote:
@mindreader https://github.com/mindreader @AnderEnder https://github.com/AnderEnder Formally NonZeroU32 (or NonZeroU16) should be used. But this extension has already many fields which could be represented with a more precise types, but are represented as String ( duration and explicit are good examples).
Personally I am ok with Strings.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rust-syndication/rss/pull/102#issuecomment-667669187, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFGHH7634ZSQQ4YSCDZP4DR6VNAJANCNFSM4PHDBQCQ .
Discovered another field I need. The item
<itunes:episode>
tag is the episode number and it corresponds to the<itunes:season>
tag that was just merged.Details here: https://help.apple.com/itc/podcasts_connect/#/itcb54353390