Skip to content
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

Major code refactoring courtesy of Gustavo #107

Closed
wants to merge 7 commits into from
Closed

Major code refactoring courtesy of Gustavo #107

wants to merge 7 commits into from

Conversation

raidancampbell
Copy link
Member

Major code changes, largely to the javascript. Some highlights are as follows:

  • Renamed a lot stuff
  • Tried to standardize some stuff
  • Removed duplicate code.
  • Removed the pooling system from NvHTTP
  • Removed the app box cache from NvHTTP

This is not ready for immediate merging, but was created for discussion as we prepare for merging.

@cgutman
Copy link
Member

cgutman commented Sep 8, 2016

Please don't merge this until I've been able to review it.

@raidancampbell
Copy link
Member Author

raidancampbell commented Sep 8, 2016

After quitting a stream, the box art isn't stylized to indicate the currently running app, and the quit current app doesn't show up. Re-entering the host selection screen shows proper behavior.

Additionally, after stopping the current app, the box art does not restylize to indicate it is no longer running

@dead
Copy link
Contributor

dead commented Sep 8, 2016

I was going to squash the commits before submit the pull request, but thanks 😄

After quitting a stream, the box art isn't stylized to indicate the currently running app, and the quit current app doesn't show up. Re-entering the host selection screen shows proper behavior.

Additionally, after stopping the current app, the box art does not restylize to indicate it is no longer running

Already fixed it but I'm gonna wait @cgutman review the code before submit a new commit.

@raidancampbell
Copy link
Member Author

@dead are you finished committing on this? I'd like to find some time within the coming weeks to fully test and merge it.

@dead
Copy link
Contributor

dead commented Sep 25, 2016

There is some stuff that I want to discuss with you guys.

  • When refresh the app list cache
  • Why/When use the hostname as the address
  • And if you guys think there is something in the code that need be changed or improved

@raidancampbell
Copy link
Member Author

Unfortunately, I let this grow stale and fall through the cracks. Sorry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants