Skip to content
This repository was archived by the owner on Dec 4, 2024. It is now read-only.

Conversation

@KOLANICH
Copy link

Also some prereq fixes.

Fixes: #3

@KOLANICH KOLANICH force-pushed the remove_node branch 5 times, most recently from cd6c352 to 087b264 Compare September 20, 2022 10:21
@fateshelled
Copy link
Owner

Thanks for pull request.
But please write an explanation. Why do you want to exclude node.js?

@KOLANICH
Copy link
Author

  1. Its ecosystem has bad packaging practices.
  2. The code written for node bundled with this lib is mostly redudndant. In the rewrite into python I got rid of most of it. If we eliminate node completely (by now I have kept that code, and allowed a user to choose, but it has some issues related to subprocessing) and the abstraction layer around a dict, we can get rid of even more, I guess the whole Netron-derived code can be excluded.
  3. The redundant code is too large to audit and auditing it is unneeded cognitive load.
  4. Requiring Node.js for a python package is ridiculous.
  5. Managing a subprocess is additional burden

Also please note: this PR has some other commits solving some other issues (after they are landed and after we get the deps into Pypi, you can publish a pypi package), but I guess they won't properly commute with the commit of denodeing, so everything is bundled.

@fateshelled
Copy link
Owner

I do not intend to make a python package.
Do what you want with your own repository.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Get rid of node.js and rewrite the part of the tool in python (or maybe in other lang, like C++).

2 participants