search

cncjs / cncjs-widget-boilerplate

Creating custom widgets for CNCjs
https://cncjs.github.io/cncjs-widget-boilerplate/
MIT License
11 stars 15 forks source link

Use default name of the CNCjs executable in README #3

Closed tawera closed 5 years ago

tawera commented 5 years ago

I noticed that the README references the name of the CNCjs executable as cnc instead of cncjs. I have changed it to make it clearer.

MitchBradley commented 5 years ago

The name of the app is currently cnc, not cncjs. You can see that by looking at https://github.com/cncjs/cncjs/blob/master/bin/cnc , or https://github.com/cncjs/cncjs/blob/master/package.json#L8

tawera commented 5 years ago

I see, I hadn't noticed because the main README refers to it as cncjs:

https://github.com/cncjs/cncjs/blob/master/README.md

But I can see there are three names for the same thing: cnc, cncjs and cncjs-server. This seems to me unfortunate but I guess there are reasons for it. I would suggest to stick to one of the names in the documentation though, so if cnc is the preferred one I suggest this should be changed in the main docs. Please feel free to reject this PR.

MitchBradley commented 5 years ago

Indeed, the change of names has been a source of confusion. The reasons are mainly due to the fact that it is difficult to foresee how a software ecosystem will evolve, thus the nomenclature one initially chooses often gets stretched to the breaking point as new components enter the mix. All of the documentation needs to be reviewed and synchronized - which unfortunately spans multiple repos and wikis.

cheton commented 5 years ago

@tawera You can find a list of symbolic links in the "bin" field of package.json, all link to ./bin/cnc:

// https://github.com/cncjs/cncjs/blob/v1.9.17/package.json#L8-L11
"bin": {
  "cnc": "./bin/cnc",
  "cncjs": "./bin/cnc",
  "cncjs-server": "./bin/cnc"
}

In the documentation, I would recommend using cncjs from the command line. I would still keep the cnc entry in v1.x just for legacy reason, but it may be deprecated when a new major version (e.g. 2.x, 3.x) is released.

Thank you for your pull request.

MitchBradley commented 5 years ago

This symlinking thing didn't work for me. There are no symlinks, only the file /bin/cnc . I think the reason for the lack of symlinks is because, on my Raspberry Pi, "npm install" crashes due to running out of memory during the "build-prod-web" step. I work around that by building on a PC and copying dist/cnc/web/* to the Raspberry Pi.

I did not realize that there is an additional step of creating symlinks. The existence of the bin/cnc file led me to think that running that file was the right way to start the server.

Having several names for the same thing is a recipe for confusion, especially when the real file is named "cnc" and the recommended name is a different symlink that is only created as the last step via npm magic.

I have been trying to support cncjs users by answering questions and helping them work through issues. The cncjs vs. cnc confusion caused a lot of wasted time during a recent help session. It was the usual EADDRINUSE problem, and when I explained that you have to interact with pm2 to stop the running server process, I told him the wrong command because of the cncjs vs. cnc ambiguity. It took several interactions over the course of hours to sort it out. We almost gave up.

My recommendation: If you think that we should be using "cncjs" to start the server, let's get rid of the other names. The script should be named cncjs and the symlink should be $PATH/cncjs . Announce it and fix the documentation so that, going forward, it is completely clear what the name is.

cheton commented 5 years ago

You are right. I forgot one thing, the symbolic link only exists when you installed cncjs via npm.

Consistent naming is really important to avoid user confusion and help answer questions effectively. I would start to show deprecation warning in next minor release 1.10, user can still start the program with cnc, but a deprecation message will appear to the console. It will be deprecated in next major release 2.0.