-
Notifications
You must be signed in to change notification settings - Fork 11
Added nox support. Fixes #10 #24
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
Conversation
Signed-off-by: Amol Kahat <amolkahat@gmail.com>
digitronik
left a comment
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.
Thanks for PR @amolkahat
I don't think we need this for CI; but yup, I would like to provide this for local development.
| - name: Unit Tests | ||
| run: py.test tests -v | ||
| - name: Run nox | ||
| run: nox |
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 would not make it as part of CI. why?
nox is just like tox which help to test package on a different version of python. Our CI already has taken care of supporting python versions and different OS.
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 added this to simplify setup and dependencies.
Currently only tests environment is added. If you want to install it we can write more code which can run with only one command like:
nox --session tests
nox --session development
nox --session user
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.
@amolkahat I like your proposal, do some changes.
- Restore workflow as it is. Still, I don't think we need
noxas part of our CI. - Add
noxdependency indev-requirements.txt - Let utilize nox features to test PR changes locally (not in CI); I mean parametrize over supported python versions like
@nox.session(python=[ "3.6", "3.7"])
def test(session):
pass
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.
@digitronik why not in CI?
| @@ -0,0 +1,8 @@ | |||
| import nox | |||
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.
For local support; we can make nox as part of dev-requirements.txt
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.
@amolkahat We also need updates to README file
digitronik
left a comment
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.
Need some changes but good to merged 👍
Thanks for nox support @amolkahat
Added nox support.
Signed-off-by: Amol Kahat amolkahat@gmail.com