Refactor date parsing#1372
Conversation
cadda5e to
380c303
Compare
| final String dateStr = playerMicroFormatRenderer.getString("uploadDate", | ||
| playerMicroFormatRenderer.getString("publishDate", "")); | ||
| if (!dateStr.isEmpty()) { | ||
| return dateStr; | ||
| } | ||
|
|
||
| final JsonObject liveDetails = playerMicroFormatRenderer.getObject( | ||
| "liveBroadcastDetails"); | ||
| if (!liveDetails.getString("endTimestamp", "").isEmpty()) { | ||
| // an ended live stream | ||
| return liveDetails.getString("endTimestamp"); | ||
| } else if (!liveDetails.getString("startTimestamp", "").isEmpty()) { | ||
| // a running live stream | ||
| return liveDetails.getString("startTimestamp"); | ||
| final var liveDetails = playerMicroFormatRenderer.getObject("liveBroadcastDetails"); | ||
| final String timestamp = liveDetails.getString("endTimestamp", // an ended live stream | ||
| liveDetails.getString("startTimestamp", "")); // a running live stream | ||
|
|
||
| if (!timestamp.isEmpty()) { | ||
| return timestamp; |
There was a problem hiding this comment.
this makes it harder to understand what's going on. I'd prefer to keep it readable and revert this change.
There was a problem hiding this comment.
I feel like the code below line 180 as a whole could be removed. I checked how the date is being extracted, and it's always taken from uploadDate or publishDate.
There was a problem hiding this comment.
YouTube might return different layouts in different cases, so it's better to keep all cases
ddaef9a to
c79afc0
Compare
|
Please list all the breaking changes in the PR description so they can be copied to the release notes. |
|
The tests are now dependent on the runner's time zone, which is not good. While the automated tests pass (server is set to UTC), tests on my computer fail: results |
|
Mmmh, what's the advantage of using |
|
0411b8a to
f38b72c
Compare
Stypox
left a comment
There was a problem hiding this comment.
Thank you! Generally this looks good to me, except for a few nitpicks
1ff529a to
afe9454
Compare
# Conflicts: # extractor/src/main/java/org/schabi/newpipe/extractor/localization/DateWrapper.java
afe9454 to
f4084ed
Compare
Co-authored-by: Stypox <stypox@pm.me>
2b784dd to
9355798
Compare
34c495a to
b3fd6b3
Compare
b3fd6b3 to
2b306a3
Compare
|
Please add this PR to the release notes and list the breaking changes in the "Breaking" section. |
This comment was marked as duplicate.
This comment was marked as duplicate.
|
@Isira-Seneviratne When running mock tests locally on my PC (in a CET timezone) I get the following failure which doesn't happen with the CI: I am specifically running all tests with mocks on commit 724cc46 from #1409 |
Thanks, I'll take a look at it. |
|
I fixed the issue here: #1411 |
|
Thank you! |
Uh oh!
There was an error while loading. Please reload this page.