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

Javascript instructions not clear #47

Closed
machinekoder opened this issue Dec 21, 2015 · 8 comments
Closed

Javascript instructions not clear #47

machinekoder opened this issue Dec 21, 2015 · 8 comments

Comments

@machinekoder
Copy link
Member

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

@bobvanderlinden
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member Author

The current model makes development of Machinetalk very hard and should therefore be changed. Maybe #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
Copy link
Member Author

@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
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member Author

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

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

No branches or pull requests

2 participants