-
Notifications
You must be signed in to change notification settings - Fork 643
Update contribution guide, move all checks to tox #1960
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
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.
Awesome
doc/development.rst
Outdated
mypy can | ||
ruff check can | ||
pylint can/**.py can/io doc/conf.py examples/**.py can/interfaces/socketcan | ||
* **pipx** is a tool for installing and running Python applications (such as tox) |
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.
Fair to aim to move entirely to uv
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.
Yeah, i guess pipx is a bit redundant now. I removed all mentions of it.
doc/development.rst
Outdated
|
||
pipx install tox | ||
|
||
**Create a virtual environment and install python-can in editable mode** |
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.
This is not standard in uv so if required it would be worth justifying.
uv run should already use that local project and the python virtual environment
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.
So far i used uv only for the speed benefit and automatic python version management on linux, so i wasn't aware of that. I updated the docs with a comparison of the traditional and the uv way. Take a look, please.
|
||
* Open a pull request from your branch to the ``main`` branch of the main python-can repository on GitHub. | ||
|
||
Please be patient — maintainers review contributions as time allows. |
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.
😅
* Add docs where appropriate. At a minimum add to ``doc/interfaces.rst`` and add | ||
a new interface specific document in ``doc/interface/*``. | ||
It should document the supported platforms and also the hardware/software it requires. | ||
A small snippet of how to install the dependencies would also be useful to get people started without much friction. |
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.
Let's steer people towards declaring optional dependencies
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 moved the attention note up and explicitely said, that pull requests for new interfaces will not be accepted. (Unless there's a good reason...)
Thank you for the unexpected review 😄 |
@hardbyte I'll merge this for now. I'm going to create another PR to introduce towncrier to automate changelog generation, so i'll need to edit the contribution guide again anyway. Then I'll request another review. |
No description provided.