Feature/issue 104 #105
Feature/issue 104 #105
Conversation
magniff
commented
Jan 9, 2019
•
magniff
commented
- Migrate to project structure, proposed at issue-104
- Tune up .travis config a bit.
Codecov Report
@@ Coverage Diff @@
## master #105 +/- ##
===========================================
- Coverage 57.19% 46.04% -11.15%
===========================================
Files 53 42 -11
Lines 1668 1303 -365
===========================================
- Hits 954 600 -354
+ Misses 714 703 -11
Continue to review full report at Codecov.
|
|
Ok, everything seems to be fine. Apparently pylint ignore path option is still broken, according to this issue pylint-dev/pylint#2541 |
There was a problem hiding this comment.
Really nice PR, thanks for this. Just two small things (primarily waiting for #106) and then LGTM. Will also let one of @zixuanzh @alexh @stuckinaboot take a look due to scope.
| - pytest --cov=./ -v | ||
| - pylint --rcfile=.pylintrc encryption host libp2p network peer stream_muxer transport tests | ||
| - pytest --cov=./libp2p tests/ | ||
| - pylint --rcfile=.pylintrc libp2p/!(kademlia|protocol_muxer) tests |
| - pip install codecov pytest pytest-cov | ||
| - pip install --upgrade pip | ||
| - pip install "pytest>=3.6" | ||
| - pip install codecov pytest-cov pytest-asyncio pylint |
There was a problem hiding this comment.
Could we have these dependencies in setup.py as well? I can't remember or find docs about why we put them here instead.
There was a problem hiding this comment.
IMO the style might be pretty suitable, or even put them in a separate requirements_dev.txt
There was a problem hiding this comment.
Ah - so we're trying to distinguish between dependencies pulled in by devs and users. That's a good idea. In that case, I think the py-evm style works well, or any suitable alternative.
There was a problem hiding this comment.
Requirements_dev is a good option, thumbs up!) I will have a chance to add it only at the late evening, though. Can we merge this code as is and add missing deps in a separate PR or something?
There was a problem hiding this comment.
Sure, see #107 for the list of changes to be made to travis.yml
|
Should be good to go once |
