Skip to content

Update frontend to Vue and rewrite backend - #39

Draft
ov3rk1ll wants to merge 2 commits into
whitewhidow:mainfrom
ov3rk1ll:vue
Draft

Update frontend to Vue and rewrite backend#39
ov3rk1ll wants to merge 2 commits into
whitewhidow:mainfrom
ov3rk1ll:vue

Conversation

@ov3rk1ll

Copy link
Copy Markdown
Contributor

This MR contains the full rewrite/change based on 2db4aab from https://github.com/ov3rk1ll/vue-sidenoder.

Nearly all files are removed or at least modified so merging this would replace the entire code base.
As discussed in #30, I'll try to split this into multiple MRs but this is the entire code to make sure nothing is missed.

@colthreepv

Copy link
Copy Markdown

what is the status of that? What needs to be done to get it out of WIP?

A simple bullet point list is enough 😉

@colthreepv colthreepv left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have glanced the PR, and I am quite pushing for this to be approved, not in the "main" branch, but in a next branch, where @whitewhidow can test and polish the branch before release

From what I have seen there is a lot of really needed work, the backend code split in files was sorely needed to keep this project maintainable

There are many details in this PR; first of all, only 2 commits please don't do that again, ever. This is so painful, and it's impossible for me (an outsider) to review the backend refactor, as it's monolithic

If the refactor was made with small commits, it could have been reviewed easily, as each commit can be read and tested individually

Random questions:

  • removed script files (windows-install.bat), what are they supposed to be replaced with?
  • I see there are patches to node_modules, what is the reason behind?

Comment thread readme.md
@@ -1,179 +1,17 @@
**SideNoder** - A **cross platform sideloader** for Quest(1&2) standalone vr headset.
# vue-sidenoder

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file needs to be reverted, with the exception of new Preview 😉

Comment thread LICENSE
@@ -1,21 +0,0 @@
MIT License

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

project needs a license file

@ov3rk1ll

Copy link
Copy Markdown
Contributor Author

@colthreepv one of the main differences is the way to two apps approach browsing:
quest-sidenoder provides a file browser that can work in any folder while vue-sidenoder only looks at the remote mount and expects folder with a certain naming scheme to work at all. While at it's core, both apps provide a similar feature set, the are more or less two different apps.

I'm working on a way to browse local files to also bring back that functionality.

There are no individual commits going from this app to vue-sidenoder as that was written completely new from scratch and this MR is just a (now outdated) rebase.
While some of the initial code was taken from this repo, almost everything in vue-sidenoder is completely rewritten at this point.

This also makes the front- and backend of both apps completely incompatible.

I'm honestly not sure what the best approach would be to go forward about pulling some changes back into this repo without a full rewrite/rebase.

@colthreepv

colthreepv commented Jan 18, 2021

Copy link
Copy Markdown

In my opinion works indeed more smoothly than the original project, not requiring side installations

Simple APK install or Folder APK install are already supported, and I think this covers base needs for users

For local file browsing I would suggest a much simpler approach, just a switch toggle between remote/local.
Remote works with naming scheme, but local does not. just simple file manager.
If we want, we could implement a File manager Vue interface, and just write the necessary API(s) to browse local files, it would be a nice-to-have that can be added later on, and with not much work, leveraging the framework, not coding the entire filebrowser by hand.

I think the decision rests on @whitewhidow, my suggestion is again to merge this PR on a next branch, make a pre-release v1.0.0 version and pick it up from there, issues can be tracked for 0.x and 1.x separately (if desired)

Just here to push this useful project in one common repository, easier to keep updated, instead of 2 projects both with pros/cons

@vikbez

vikbez commented Jan 23, 2021

Copy link
Copy Markdown

Thanks for the rewrite @ov3rk1ll this looks and works really smoothly. I hope it gets merged somehow !

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