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

Feat: support project runner #277

Merged

Conversation

viralgupta
Copy link
Contributor

@viralgupta viralgupta commented Aug 10, 2024

Supported to init, run, build Neutralino Apps with project runner

  • supports neu config parameters like so under cli
    "projectRunner": {
      "projectPath": "/",
      "buildPath": "./node-src/dist/",
      "initCommand": "npm run runner:install",  
      "devCommand": "npm run runner:start",     
      "buildCommand": "npm run runner:build"
    }
  • initCommand, devCommand, buildCommand are run inside root of the Neu App
  • could not understand the need of type attribute so did not include it

Todo: (in this repo)

  • add logic for running frontend library
  • run binaries with new name when in build mode

Copy link
Member

@shalithasuranga shalithasuranga left a comment

Choose a reason for hiding this comment

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

Thanks so much for your contribution @viralgupta 🎉

@shalithasuranga shalithasuranga merged commit 5827fca into neutralinojs:main Aug 19, 2024
2 checks passed
Copy link
Contributor

@CosmoMyzrailGorynych CosmoMyzrailGorynych left a comment

Choose a reason for hiding this comment

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

I don't think this approach is good in the long term. Neutralino.js is a simple framework that has its precise purpose and I would rather leave it be that way. Any framework built on top of Neutralino.js should use it programmatically and sort its build files on its own, and provide its own CLIs to run apps. There are also options like using npm scripts instead of expanding neutralino.js API.

About the code itself,
The bin folder is hard-coded (bad, needs configs as future frameworks may already use the bin folder) and the code for moving the neutralino files could instead be merged with the loop above to simplify the code and remove excess file operations.
Also, please add code comments. The 'Copying Project Runner build...' part reflects just one line that does exactly that in the whole if branch.

@shalithasuranga
Copy link
Member

Thanks so much for your feedback @CosmoMyzrailGorynych. Please note that this PR simply lets a GSoC contributor run his community project idea (named NodeNeutralino). This is directly merged but improved with 4987353 as the GSoC contributor's main focus is NodeNeutralino - not working with the CLI.

This hostProject (previously projectRunner) delivers a concept where someone can use Neutralinojs with custom backends (not extensions). The idea is explained in official docs. We would like to see PyNeutralino, GoNeutralino, etc. they are just bindings for Neutralino ;)

Thanks 🎉

@CosmoMyzrailGorynych
Copy link
Contributor

I understood the concept of this PR, that is is about NeutralinoNode, yet my points still stand. Tailoring a more general-use framework towards one use case is not a good idea imo.

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