-
-
Notifications
You must be signed in to change notification settings - Fork 490
Refactor date parsing #1372
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
Refactor date parsing #1372
Conversation
cadda5e
to
380c303
Compare
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.
thanks
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YouTube might return different layouts in different cases, so it's better to keep all cases
...in/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeStreamExtractor.java
Outdated
Show resolved
Hide resolved
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
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.
Thank you! Generally this looks good to me, except for a few nitpicks
extractor/src/main/java/org/schabi/newpipe/extractor/localization/TimeAgoParser.java
Show resolved
Hide resolved
extractor/src/main/java/org/schabi/newpipe/extractor/utils/ExtractorHelper.java
Outdated
Show resolved
Hide resolved
...ctor/src/main/java/org/schabi/newpipe/extractor/services/peertube/PeertubeParsingHelper.java
Show resolved
Hide resolved
...a/org/schabi/newpipe/extractor/services/soundcloud/extractors/SoundcloudStreamExtractor.java
Show resolved
Hide resolved
.../src/main/java/org/schabi/newpipe/extractor/services/soundcloud/SoundcloudParsingHelper.java
Outdated
Show resolved
Hide resolved
...abi/newpipe/extractor/services/youtube/extractors/kiosk/YoutubeChartsBaseKioskExtractor.java
Outdated
Show resolved
Hide resolved
...in/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeStreamExtractor.java
Outdated
Show resolved
Hide resolved
...org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeStreamInfoItemExtractor.java
Show resolved
Hide resolved
extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/YoutubeParsingHelper.java
Show resolved
Hide resolved
1ff529a
to
afe9454
Compare
# Conflicts: # extractor/src/main/java/org/schabi/newpipe/extractor/localization/DateWrapper.java
afe9454
to
f4084ed
Compare
Co-authored-by: Stypox <[email protected]>
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. |
Uh oh!
There was an error while loading. Please reload this page.