Skip to content
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

docs: CONTRIBUTING.md #547

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pfackeldey
Copy link
Collaborator

No description provided.

@pfackeldey pfackeldey changed the title Add CONTRIBUTING.md docs: CONTRIBUTING.md Oct 7, 2024
Copy link
Collaborator

@martindurant martindurant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CONTRIBUTING.md Outdated
Install the development environment (once) with

```bash
uv venv
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python -m venv

rather than assume users have uv (or conda or whatever)

CONTRIBUTING.md Outdated
Source development environment with

```bash
source .venv/bin/activate
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this need to come before pip install?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general yes, but not with uv. If it finds a venv (even though it is not activated) it will install software into it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so if we remove requiring uv, then this will need to be reordered. That's conceptually cleaner anyway, if you ask me.

CONTRIBUTING.md Outdated
Run tests with

```bash
pytest .
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pytest .
pytest -v

CONTRIBUTING.md Outdated
Build documentation with

```bash
cd docs && make html
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does && work on windows?

Also worth saying where the docs end up.

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.00%. Comparing base (8cb8994) to head (ec12efe).
Report is 148 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #547      +/-   ##
==========================================
- Coverage   93.06%   93.00%   -0.07%     
==========================================
  Files          23       22       -1     
  Lines        3290     3387      +97     
==========================================
+ Hits         3062     3150      +88     
- Misses        228      237       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pfackeldey
Copy link
Collaborator Author

Thank you very much for pointing to the contribution guide in the documentation - I didn't see it!
I updated this PR and linked it directly in the README.md; I hope this sounds reasonable to you.

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

Successfully merging this pull request may close these issues.

3 participants