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

Add wayland and windows start options #3594

Merged
merged 6 commits into from
Oct 23, 2024

Conversation

KristjanESPERANTO
Copy link
Contributor

@KristjanESPERANTO KristjanESPERANTO commented Oct 23, 2024

This PR adds start options for Wayland and Windows.

This would solve issue #3582.

To Do if this PR is merged:

@KristjanESPERANTO KristjanESPERANTO changed the title Add wayland and windows options Add wayland and windows start options Oct 23, 2024
@KristjanESPERANTO
Copy link
Contributor Author

I dropped the attempt to add an windows installation option for this PR.

@KristjanESPERANTO KristjanESPERANTO marked this pull request as ready for review October 23, 2024 19:22
@sdetweil
Copy link
Collaborator

@KristjanESPERANTO if you used my shell js app it could be on install-mm

@KristjanESPERANTO
Copy link
Contributor Author

@KristjanESPERANTO if you used my shell js app it could be on install-mm

I think it's good to handle this in different PRs. This one handles the start options and a new one could handle the installation.

@khassel khassel merged commit aa7e856 into MagicMirrorOrg:develop Oct 23, 2024
7 of 11 checks passed
@khassel
Copy link
Collaborator

khassel commented Oct 23, 2024

btw tested this in VirtualBox

"install-mm:windows": "cmd /c npm install --ignore-scripts --no-audit --no-fund --no-update-notifier --only=prod --omit=dev && echo 'Installing vendor files ...' && cd vendor && npm install --loglevel=error --no-audit --no-fund --no-update-notifier && echo 'Installing fonts ...' && cd fonts && npm install --loglevel=error --no-audit --no-fund --no-update-notifier",`

and beside a small error (should be cd ../fonts) it worked :)

@sdetweil
Copy link
Collaborator

i think this could have been done together

my postinstall using my launcher

"postinstall": "node js/es.js \"npm run install-vendor && npm run install-fonts && echo MagicMirror² installation finished successfully!\n\":npm run install-vendor & npm run install-fonts & echo MagicMirror² installation finished successfully!",

then we don't need multiple divergent commands.. (to document and support)

@sdetweil
Copy link
Collaborator

sdetweil commented Oct 23, 2024

then we just use run mm-install
if you ran install, then postinstall would be the same

@sdetweil
Copy link
Collaborator

sdetweil commented Oct 23, 2024

we can do the same on start too..

js/es.js linux_command : windows_command

less confusing for our users..

KristjanESPERANTO added a commit to KristjanESPERANTO/MagicMirror-Documentation that referenced this pull request Oct 23, 2024
@KristjanESPERANTO
Copy link
Contributor Author

we can do the same on start too..

js/es.js linux_command : windows_command

We would need something like this: js/exec_script.js x11_command : wayland_command : windows_command.

less confusing for our users..

Yes, but different start options should not overwhelm a tinkerer. Also, when someone uses the additional commands, they should always be reminded that they are doing something that is not the default. This might help them to solve problems themselves and need less support.

Perhaps I will argue in a different direction in the future 😅. But I often have the thought that it might not be a good idea to make it too easy for our users (who are tinkerers). If we automate everything, they understand the system less and need more support. That's why I liked the decision that only the manual installation is the official one.

rejas pushed a commit to MagicMirrorOrg/MagicMirror-Documentation that referenced this pull request Oct 25, 2024
@KristjanESPERANTO KristjanESPERANTO deleted the wayland-windows branch October 29, 2024 14:13
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