search

jellyfin / jellyfin-roku

The Official Roku Client for Jellyfin
https://jellyfin.org
GNU General Public License v2.0
412 stars 128 forks source link

Fix compile warnings #1112

Open cewert opened 1 year ago

cewert commented 1 year ago

Is your feature request related to a problem? Please describe

While testing the latest version of unstable, I noticed there are several compile warnings displayed when deploying the app in vscode (hitting F5).

I don't understand why the app appears to be compiled twice with different warnings and compile times.

I also don't understand why we are seeing warnings for unused variables during compile time when our CI is supposed to be flagging unused variables as errors

Describe the solution you'd like

Only compile once and have zero compile warnings

Additional context

------ Compiling dev 'Jellyfin' ------

BRIGHTSCRIPT: WARNING: unused variable 'routestartpositionticks' in anonymous function in pkg:/source/roku_modules/api/api.brs(2431)

BRIGHTSCRIPT: WARNING: unused variable 'url' in function 'logger__setserverurl' in pkg:/source/testFramework/UnitTestFramework.brs(1037)
BRIGHTSCRIPT: WARNING: unused variable 'item' in function 'tf_utils__findelementindexinarray' in pkg:/source/testFramework/UnitTestFramework.brs(2735)
BRIGHTSCRIPT: WARNING: unused variable 'tracks' in function 'selectsubtitletrack' in pkg:/source/utils/Subtitles.brs(114)
BRIGHTSCRIPT: WARNING: unused variable 'current' in function 'selectsubtitletrack' in pkg:/source/utils/Subtitles.brs(114)
BRIGHTSCRIPT: WARNING: unused variable 'id' in function 'sortsubtitles' in pkg:/source/utils/Subtitles.brs(190)
BRIGHTSCRIPT: WARNING: unused variable 'press' in function 'onkeyevent' in pkg:/components/ItemGrid/ItemGridOptions.brs(229)
BRIGHTSCRIPT: WARNING: unused variable 'fully_external' in function 'addvideocontenturl' in pkg:/components/ItemGrid/LoadVideoContentTask.brs(176)
BRIGHTSCRIPT: WARNING: unused variable 'id' in function 'sortsubtitles' in pkg:/components/ItemGrid/LoadVideoContentTask.brs(602)
BRIGHTSCRIPT: WARNING: unused variable 'key' in function 'onkeyevent' in pkg:/components/JFGroup.brs(4)
BRIGHTSCRIPT: WARNING: unused variable 'press' in function 'onkeyevent' in pkg:/components/JFMessageDialog.brs(9)
BRIGHTSCRIPT: WARNING: unused variable 'msg' in function 'onstate' in pkg:/components/JFVideo.brs(142)
BRIGHTSCRIPT: WARNING: unused variable 'msg' in function 'buffercheck' in pkg:/components/JFVideo.brs(221)
BRIGHTSCRIPT: WARNING: unused variable 'press' in function 'onkeyevent' in pkg:/components/PlaybackDialog.brs(1)
BRIGHTSCRIPT: WARNING: unused variable 'e' in function 'processclientdiscoveryresponse' in pkg:/components/config/ServerDiscoveryTask.brs(65)
BRIGHTSCRIPT: WARNING: unused variable 'key' in function 'onkeyevent' in pkg:/components/login/UserRow.brs(49)
BRIGHTSCRIPT: WARNING: unused variable 'press' in function 'onkeyevent' in pkg:/components/movies/MovieOptions.brs(97)
BRIGHTSCRIPT: WARNING: unused variable 'press' in function 'onkeyevent' in pkg:/components/search/SearchResults.brs(42)
BRIGHTSCRIPT: WARNING: unused variable 'key' in function 'onkeyevent' in pkg:/components/tvshows/TVEpisodeRow.brs(58)
BRIGHTSCRIPT: WARNING: unused variable 'press' in function 'onkeyevent' in pkg:/components/tvshows/TVListOptions.brs(71)
BRIGHTSCRIPT: WARNING: unused variable 'msg' in function 'onstate' in pkg:/components/video/VideoPlayerView.brs(147)
BRIGHTSCRIPT: WARNING: unused variable 'msg' in function 'buffercheck' in pkg:/components/video/VideoPlayerView.brs(227)
Displayed 22 of 22 warnings
03-20 19:27:35.733 [scrpt.ctx.cmpl.time] Compiled 'Jellyfin', id 'dev' in 896 milliseconds (BCVer:0)

03-20 19:27:35.740 [scrpt.proc.mkup.time] Processed markup dev 'Jellyfin' in 1 milliseconds

------ Compiling dev 'Jellyfin' ------

BRIGHTSCRIPT: WARNING: unused variable 'routestartpositionticks' in anonymous function in pkg:/source/roku_modules/api/api.brs(2431)

BRIGHTSCRIPT: WARNING: unused variable 'url' in function 'logger__setserverurl' in pkg:/source/testFramework/UnitTestFramework.brs(1037)
BRIGHTSCRIPT: WARNING: unused variable 'item' in function 'tf_utils__findelementindexinarray' in pkg:/source/testFramework/UnitTestFramework.brs(2735)
BRIGHTSCRIPT: WARNING: unused variable 'tracks' in function 'selectsubtitletrack' in pkg:/source/utils/Subtitles.brs(114)
BRIGHTSCRIPT: WARNING: unused variable 'current' in function 'selectsubtitletrack' in pkg:/source/utils/Subtitles.brs(114)
BRIGHTSCRIPT: WARNING: unused variable 'id' in function 'sortsubtitles' in pkg:/source/utils/Subtitles.brs(190)
Displayed 6 of 6 warnings
03-20 19:27:35.913 [scrpt.ctx.cmpl.time] Compiled 'Jellyfin', id 'dev' in 172 milliseconds (BCVer:0)
cewert commented 1 year ago

One of the unused variables is from a roku module that we import during npm install - the api module.

This issue points out the downside of using roku modules - we lose control of them and they don't have to pass through our CI checks. At the very least we should be using specific versions of these modules and only upgrading them once we approve of the changes. Right now though, these modules are set to automatically update to the latest minor/build version without any approval (that's what the caret means in package.json).

This seems like a huge design flaw in our app IMO. The roku modules aren't owned by the Jellyfin org and present a potential security risk if the repos were to fall in the wrong hands. It also makes it so we can't run or test the app without running npm install or else we will be missing files.

Edit: What are the benefits to us using roku modules for our app? Like I know the NPM modules could potentially help other people make a Roku app, but what is the advantage to us? I don't see any advantages only downsides.