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

Add AA to _applicationsCtxtWiseAppNames for v59+ #117

Merged
merged 1 commit into from Jan 4, 2018

Conversation

Projects
None yet
2 participants
@Trevelopment
Collaborator

Trevelopment commented Dec 30, 2017

In v59 there is an additional array that is checked when populating the app list. Before this was solved by patching systemApp.js but this solves the issue without the need for a patch.

Add AA to _applicationsCtxtWiseAppNames for v59+
In v59 there is an additional array that is checked when populating the app list.  Before this was solved by patching systemApp.js but this solves the issue without the need for a patch.

@Trevelopment Trevelopment requested a review from lmagder Dec 30, 2017

@Trevelopment

This comment has been minimized.

Show comment
Hide comment
@Trevelopment

Trevelopment Dec 30, 2017

Collaborator

Around the time I took over the AIO project v59 was just coming out and one of my first challenges was that apps would not show up in the applications list for v59. I found out by looking at the code that they had added 2 arrays of accepted app names and what "context" they belong to which all that did was check the app names against the arrays and only add the apps whos names were in one of those arrays, 3 of the apps in one array go into a "vsm" submenu and the rest in the root applications menu. So I originally came up with a patch to solve this issue by commenting out the check against the arrays in systemApp.js but the side effect of that was the apps under the vsm submenu (a very stupid thing, who needs a submenu with only 3 or 4 apps?!) would show up in the root applications menu but any app added would show up in the list since there was no more check (BTW I think they added this for the sole purpose of preventing use of custom apps). I got so many complaints about that (I have no idea why people wanted that submenu so badly) so I reworked the patch to literally concat an array of app names that I came up with (pretty much every app I had ever heard of) to the array of accepted apps. That worked for a long time and pretty much still does (as long as you use AIO to install apps it patches systemApp.js automatically) since most people only add the apps from AIO and this one but I have been messing around with CASDK lately and in doing that I came across some new apps and with my patch they wouldn't show up in the list because I had not put their app names in the array. So I took another look at additionalApps.js and realized we can dynamically add app names to that array as we register them and that solves the issue 100% of the time, no patch needed.

Collaborator

Trevelopment commented Dec 30, 2017

Around the time I took over the AIO project v59 was just coming out and one of my first challenges was that apps would not show up in the applications list for v59. I found out by looking at the code that they had added 2 arrays of accepted app names and what "context" they belong to which all that did was check the app names against the arrays and only add the apps whos names were in one of those arrays, 3 of the apps in one array go into a "vsm" submenu and the rest in the root applications menu. So I originally came up with a patch to solve this issue by commenting out the check against the arrays in systemApp.js but the side effect of that was the apps under the vsm submenu (a very stupid thing, who needs a submenu with only 3 or 4 apps?!) would show up in the root applications menu but any app added would show up in the list since there was no more check (BTW I think they added this for the sole purpose of preventing use of custom apps). I got so many complaints about that (I have no idea why people wanted that submenu so badly) so I reworked the patch to literally concat an array of app names that I came up with (pretty much every app I had ever heard of) to the array of accepted apps. That worked for a long time and pretty much still does (as long as you use AIO to install apps it patches systemApp.js automatically) since most people only add the apps from AIO and this one but I have been messing around with CASDK lately and in doing that I came across some new apps and with my patch they wouldn't show up in the list because I had not put their app names in the array. So I took another look at additionalApps.js and realized we can dynamically add app names to that array as we register them and that solves the issue 100% of the time, no patch needed.

@lmagder

lmagder approved these changes Jan 3, 2018

Looks good to me. I honestly only have a pretty superficial knowledge about this part of the code, it was mostly here when I got here, but your explanation makes sense.

You're the expert with the JS-side of stuff so I say go for it even if I can't test it 😃 Supporting new versions is great, especially when it doesn't require drastic changes.

@Trevelopment

This comment has been minimized.

Show comment
Hide comment
@Trevelopment

Trevelopment Jan 4, 2018

Collaborator

@lmagder Hey man I am so dumb but you may think this is funny. I thought I had like corrupted my disk or something because the headunit files would not copy over anymore after I had uninstalled everything for a day or 2. And I was flipping out a bit because only the headunit_libs folder copied over and I tried like 3 times and deleted the whole bin folder one of the times and remade it still headunit and the wrapper would not copy over. So all day in between working I'm researching corrupted disks and fscks and udevs just all kinds of stuff that has to do with filesystems and wondering what I did to do that and just learning all kinds of random stuff all day. Then I go to check it and I unmount the partition and check it e2fsck -c and play around with the new video player app and do all that and I'm like what the hell it's all fine but then I run a quick df -h and see it right away... /dev/whateveritis 538.2 538.2 100% /tmp/mnt/data_persist and then I realize how dumb I am... 520MB of dump files later we gonna get that 100% down to 2%!! MErge TimE!

Collaborator

Trevelopment commented Jan 4, 2018

@lmagder Hey man I am so dumb but you may think this is funny. I thought I had like corrupted my disk or something because the headunit files would not copy over anymore after I had uninstalled everything for a day or 2. And I was flipping out a bit because only the headunit_libs folder copied over and I tried like 3 times and deleted the whole bin folder one of the times and remade it still headunit and the wrapper would not copy over. So all day in between working I'm researching corrupted disks and fscks and udevs just all kinds of stuff that has to do with filesystems and wondering what I did to do that and just learning all kinds of random stuff all day. Then I go to check it and I unmount the partition and check it e2fsck -c and play around with the new video player app and do all that and I'm like what the hell it's all fine but then I run a quick df -h and see it right away... /dev/whateveritis 538.2 538.2 100% /tmp/mnt/data_persist and then I realize how dumb I am... 520MB of dump files later we gonna get that 100% down to 2%!! MErge TimE!

@Trevelopment Trevelopment merged commit 96d3845 into gartnera:master Jan 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment