Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Awesome Endeavour - unsafe-eval no more! #991

Closed
11 tasks done
dignifiedquire opened this issue Sep 4, 2017 · 17 comments
Closed
11 tasks done

Awesome Endeavour - unsafe-eval no more! #991

dignifiedquire opened this issue Sep 4, 2017 · 17 comments

Comments

@dignifiedquire
Copy link
Member

dignifiedquire commented Sep 4, 2017

@daviddias daviddias changed the title No more unsafe eval Awesome Endeavour - unsafe-eval no more! Sep 5, 2017
@daviddias
Copy link
Member

Code coverage is dropping dramatically with this change because it is trying to check if we test the newly generated code:

@dignifiedquire
Copy link
Member Author

Just published a release of aegir which allows to ignore files from coverage: aegir coverage --ignore src/somefile.js

@daviddias
Copy link
Member

Needs this to get done - ipld/js-ipld-dag-pb#41

@dignifiedquire
Copy link
Member Author

Good news, we don't need to migrate, just use this version of protocol-buffers I made which switches to regular functions instead of generating them and evaling:

https://github.com/dignifiedquire/protocol-buffers/tree/fixes

perf is a bit worse, but not that bad

npm (encode) x 482,202 ops/sec ±3.14% (85 runs sampled)
npm (decode) x 1,863,500 ops/sec ±1.23% (91 runs sampled)
npm(encode + decode) x 349,692 ops/sec ±1.66% (82 runs sampled)
local (encode) x 423,864 ops/sec ±1.51% (88 runs sampled)
local (decode) x 1,042,350 ops/sec ±1.19% (94 runs sampled)
local(encode + decode) x 294,290 ops/sec ±1.39% (89 runs sampled)

@daviddias
Copy link
Member

Awesome @dignifiedquire !

Might make it a real module and not a fork (i.e maintained, CI, published to npm, etc)?

@dignifiedquire
Copy link
Member Author

Probably yes

@dignifiedquire
Copy link
Member Author

Renamed to protons: https://github.com/dignifiedquire/protons publish coming soon

@dignifiedquire
Copy link
Member Author

Just published https://www.npmjs.com/package/protons

@daviddias
Copy link
Member

Awesome. Make sure to have a reference in the README to the original module and the reason that lead to the fork, other people will find useful.

@dignifiedquire
Copy link
Member Author

there already is?

dignifiedquire added a commit to libp2p/js-libp2p-crypto that referenced this issue Sep 7, 2017
dignifiedquire added a commit to ipld/js-ipld-dag-pb that referenced this issue Sep 7, 2017
dignifiedquire added a commit to ipfs/js-ipfs-unixfs that referenced this issue Sep 7, 2017
dignifiedquire added a commit to ipfs/js-ipfs-bitswap that referenced this issue Sep 7, 2017
dignifiedquire added a commit to libp2p/js-libp2p-identify that referenced this issue Sep 7, 2017
dignifiedquire added a commit to libp2p/js-libp2p-record that referenced this issue Sep 7, 2017
dignifiedquire added a commit to libp2p/js-libp2p-secio that referenced this issue Sep 7, 2017
@dignifiedquire
Copy link
Member Author

@diasdavid PRs are ready for you to review

daviddias pushed a commit to libp2p/js-libp2p-crypto that referenced this issue Sep 7, 2017
daviddias pushed a commit to libp2p/js-libp2p-identify that referenced this issue Sep 7, 2017
* feat: replace protocol-buffers with protons

Ref ipfs/js-ipfs#991

* feat: upgrade to aegir@12
daviddias pushed a commit to libp2p/js-libp2p-secio that referenced this issue Sep 7, 2017
daviddias pushed a commit to libp2p/js-libp2p-record that referenced this issue Sep 7, 2017
* feat: replace protocol-buffers with protons

Ref ipfs/js-ipfs#991

* feat: upgrade to aegir@12

* chore: fix coverage command
daviddias pushed a commit to ipld/js-ipld-dag-pb that referenced this issue Sep 7, 2017
daviddias pushed a commit to ipfs/js-ipfs-unixfs that referenced this issue Sep 7, 2017
* feat: upgrade to new aegir

* feat: replace protocol-buffers with protons

Ref ipfs/js-ipfs#991

* chore: fix travis config
daviddias pushed a commit to ipfs/js-ipfs-bitswap that referenced this issue Sep 7, 2017
@daviddias
Copy link
Member

Ok, everything merged, released and all the deps in the middle were also updated to make sure that npm always installs the latest patch version.

Test away :)

@daviddias
Copy link
Member

Tracking results:

@dignifiedquire
Copy link
Member Author

Missed two modules

npm ls protocol-buffers
ipfs@0.25.4 /Users/dignifiedquire/opensource/ipfs/js-ipfs
├─┬ libp2p-floodsub@0.11.0
│ └─┬ libp2p-crypto@0.9.4
│   └── protocol-buffers@3.2.1  deduped
└─┬ libp2p-kad-dht@0.5.0
  └── protocol-buffers@3.2.1

dignifiedquire added a commit to libp2p/js-libp2p-kad-dht that referenced this issue Sep 7, 2017
dignifiedquire added a commit to libp2p/js-libp2p-floodsub that referenced this issue Sep 7, 2017
daviddias pushed a commit to libp2p/js-libp2p-kad-dht that referenced this issue Sep 7, 2017
daviddias pushed a commit to libp2p/js-libp2p-floodsub that referenced this issue Sep 7, 2017
* feat: replace protocol-buffers with protons

Ref ipfs/js-ipfs#991

* chore: ignore lockfiles
mkg20001 added a commit to ZeroNetJS/zeronet-js that referenced this issue Sep 7, 2017
@daviddias
Copy link
Member

daviddias commented Sep 7, 2017

> npm ls protocol-buffers
ipfs@0.25.4 /Users/imp/code/js-ipfs
└── (empty)

@dignifiedquire
Copy link
Member Author

Closing?

@daviddias
Copy link
Member

image

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

No branches or pull requests

2 participants