machinekit / machinetalk-protobuf

Protobuf declarations for machinekit messages
MIT License
10 stars 11 forks source link

Javascript instructions not clear #47

Closed machinekoder closed 8 years ago

machinekoder commented 8 years ago

How to install machinetalk-protobuf for nodejs is not clear. The instructions are probably outdated (newest version of protobuf.js uses pbjs).

bobvanderlinden commented 8 years ago

I'd like to drop generating a NodeJS library in this repository and let people use machinetalk-protobuf from npm:

npm install machinetalk-protobuf

That package's Git repository can be found here: https://github.com/bobvanderlinden/machinetalk-protobuf-node.

It allows people to add a dependency from other node projects to the protobuf definitions, for example: https://github.com/bobvanderlinden/node-machinetalk/blob/master/package.json#L14

machinekoder commented 8 years ago

But it splits up development into multiple projects which is not what we want. Adding machinetalk protobuf to npm or pip is fine as long as it is generated from the main repository. Clear instructions need to be defined how to make a local build.

bobvanderlinden commented 8 years ago

It splits up development the same way Machinekit does at the moment. The protobuf definitions are directly from machinetalk-protobuf using git subtree to merge/update the files. The instructions on how to change/update the protobuf definitions are here: https://github.com/bobvanderlinden/machinetalk-protobuf-node

If we were to merge the node package inside machinetalk-protobuf, it is required to put package.json in the root of the repository (or else there isn't a way to install it using Git that I know of). I can see this being a better approach, but it does mean that the machinetalk-protobuf will become somewhat messier (package.json, .npmignore and maybe others). Versioning of machinekit-protobuf should also be handled more strictly this way: the same version can never be published twice on npmjs. Additionally, when publishing it always needs a version. At the moment I would this for machinetalk-protobuf-node, but that requirement would then propagate to machinetalk-protobuf.

machinekoder commented 8 years ago

The current model makes development of Machinetalk very hard and should therefore be changed. Maybe https://github.com/machinekit/machinetalk-protobuf/issues/42 could help. Machinetalk must get easier to use and to extend to increase the community. Not sure how we could handle package distribution of the definitions.

machinekoder commented 8 years ago

@bobvanderlinden I tried installing machinetalk-protobuf with npm and indeed that is very easy. However, we still need an easy way to setup a development environment where one can test different bindings easily. git subtree is not very useful for handling this problem. Could your repository somehow be extended to generate the nodejs files from protobuf descriptions located in the system path if present?

machinekoder commented 8 years ago

The point is that if I modify the protobuf file to create a new protocol I would need to run git subtree for every binding that uses it. If a global machinetalk-protobuf installation is used I would just need to hit the recompile button (or use entr to automatically retrigger a build)

bobvanderlinden commented 8 years ago

Generating nodejs files from files gathered from the system (and not the repo) will result in different people generating different packages with the same version. It's not something you'd want. Additionally it creates problems with CI.

PR #48 should help with this. I think it's a step in the direction (no more out-of-sync npm vs protobuf).

I was thinking of adding some tests to know that a few basic values in the protobuf messages are actually available in Node. Additionally adding Travis to automatically test, package and publish to npm when new versions are tagged. Same could be done for Pyhon.

machinekoder commented 8 years ago

JavaScript instructions are now clear. I will close this issue.