-
Notifications
You must be signed in to change notification settings - Fork 75
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
Travis ci integration #25
Conversation
@hellais @guyharris some reasonable changes here.. so review when you get a chance. The docs will be automatically generated and placed here: http://pypcap.readthedocs.io/en/latest/ Coveralls is really neato.. you can drill down to individual files and see exactly which lines are not covered: https://coveralls.io/builds/6837945/source?filename=chains%2Fsources%2Fpacket_streamer.py |
This looks great! Sorry for not checking it out sooner. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few comments and some minor required fixes to have this merged.
@@ -1,6 +1,8 @@ | |||
Python pcap module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that this is in .rst format we may want to also read it and import it for display on pypi.
@@ -0,0 +1,10 @@ | |||
======= | |||
Class 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these names dummy names that are going to change?
|
||
def get_version(filename): | ||
# Hack this for right now since pypcap doesn't have standard module setup | ||
return '1.1.5' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh.
Yeah we should come up with some better system for managing versioning, maybe one way to go is to create a simple shim around the c extension that we call _pypcap and have a pure python wrapper around it that is the actual pypcap.
Another way to do it would be to put version information in a variable in our setup.py and import it from there.
# Mock any objects that we might need to | ||
from mock import Mock as MagicMock | ||
|
||
class Mock(MagicMock): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't MagicMock already part of the mock module?
- Go to github and hit 'New pull request' | ||
- Someone reviews it and says 'AOK' | ||
- Merge the pull request (green button) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
|
||
* libpcap-dev | ||
|
||
* python-dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we start uploading also wheels we could also add a comment that these are not needed for the distributions where we support wheels.
test_release = sdist bdist_wheel upload -r pypitest | ||
|
||
[metadata] | ||
description-file = README.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be "Readme.rst"
test_pcap_properties() | ||
test_pcap_errors() | ||
test_pcap_dispatch() | ||
test_pcap_readpkts() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you no longer wrapping things inside of the unittest class?
deps = -rdocs/requirements.txt | ||
changedir=docs | ||
commands = sphinx-build -b html -d {envtmpdir}/doctrees . {envtmpdir}/html | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
I implemented the suggested fixes on a copy of this branch. |
No description provided.