-
Notifications
You must be signed in to change notification settings - Fork 420
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
Improve channel tabs code #1092
Conversation
Shouldn't you apply the same changes for next pages too? |
c8c5ff5
to
07bb752
Compare
Right, my bad. Fixed. |
The commit message isn't though. |
Note that this introduces a "Raw use of parameterized class 'InfoItemsPage'" warning, but it can be ignored since the type missing would be <InfoItem>, and StreamInfoItem extends InfoItem
07bb752
to
6d22271
Compare
Fixed |
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 for these improvements and for this one line saving!
I implemented my suggestions from #1082 here, read the commit subjects for specific info. This completes the channel tabs PR, but can be merged with less priority.
I basically rewrote from scratch the channel tab tests, as you can simply inherit
DefaultListExtractorTest
and let the default implementation do the rest. All tests still pass. Furthermore, I split the age restricted channel tabs test into two tests, although this implies the common channel page is fetched twice.We still need to decide whether we want to rename
ReadyChannelTabListLinkHandler
to something else. tldr it's a class that is used as a temporary ListLinkHandler which can only be instantiated internally (while other link handlers are instantiatable by anyone) in the extractor for specific purposes, e.g. to prevent additional fetches of data in channel tab extractors when such data has already been extracted in the channel extractor.Let me know if e34739e is ok in your opinion. There is now a "Raw use of parameterized class 'InfoItemsPage'" warning, but it can be ignored since the type missing would be
<InfoItem>
, andStreamInfoItem extends InfoItem
.