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 NodeJS server as an alternative to PHP #136

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

limaanto
Copy link

Problem

I wanted to have an app similar to my_webapp but with nodejs instead of PHP.
After hesitating to create a new app, I decided it was better to improve this one instead.

Solution

For this, I added, with the help of @oiseauroch:

  • Two entries in the manifest:
    • A choice for the NodeJS version
    • A port (provisionned in any case due to ynh limitations, but this should not matter)
  • Services and configs:
    • Systemctl configs to run the NodeJS server
    • A watcher service and path to restart NodeJS upon file update
    • A custom NGinx config because it is incompatible with the default one
  • Docs:
    • More info in the description and admin

The install/remove/backup/restore have been adapted and tested.
The upgrade script is updated but not tested
The change_url script does not change

It is not possible to have both PHP and NodeJS to keep the scripts simple.

PR Status

  • Code finished and ready to be reviewed/tested
  • The fix/enhancement were manually tested (if applicable)

Automatic tests

Automatic tests can be triggered on https://ci-apps-dev.yunohost.org/ after creating the PR, by commenting "!testme", "!gogogadgetoci" or "By the power of systemd, I invoke The Great App CI to test this Pull Request!". (N.B. : for this to work you need to be a member of the Yunohost-Apps organization)

This adds:
- Two entries in the manifest:
    - A choice for the NodeJS version
    - A port (provisionned in any case due to ynh limitations, but this should not matter)
- Services and configs:
    - Systemctl configs to run the NodeJS server
    - A watcher service and path to restart NodeJS upon file update
    - A custom NGinx config because it is incompatible with the default one
- Docs:
    - More info in the description and admin

The install/remove/backup/restore have been adapted and tested.
The upgrade script is updated but not tested
The change_url script does not change

It is not possible to have both PHP and NodeJS to keep the scripts simple.
@oiseauroch
Copy link

!testme

@yunohost-bot
Copy link
Contributor

Fingers crossed!
Test Badge

@oiseauroch
Copy link

!testme

@yunohost-bot
Copy link
Contributor

Alrighty!
Test Badge

@alexAubin
Copy link
Member

This sounds cool but i don't know what to think about this ... This raises the question of what really is the scope of this app. Somebody could also add support for Python, Ruby or whatever and we end up in Feature creep hell

To me the support for PHP+SQL db should already justify splitting the half in two apps, once for pure static hosting (possibly with Git actions in the config panel) and the other one for PHP(+SQL). There's also the Flask app, which is somewhat similar but for Python ... Imho we should really discuss wether we want a single my_webapp that fits every use case, or several "my__webapp" that are skeletons that are design for specific technologies

@limaanto
Copy link
Author

Hi! Thanks for answering so quickly
I agree that this is not as clean as I first though. Just with nodejs, I saw a lot of edge cases that were hard to keep track of, and this will only get worse.
Maybe it was a mistake to directly open a PR and we should first discuss this in a dedicated issue. In this case, I'll be happy to participate in the discussion :)

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.

4 participants