|
| 1 | +# Contributing |
| 2 | + |
| 3 | +Thanks for looking. libvcs accepts contributions through |
| 4 | +[GitHub](https://github.com/vcs-python/libvcs). libvcs is pre-1.0: bug |
| 5 | +reports with a reproduction, and reports of where the API or the |
| 6 | +documentation misled you, are the most useful contributions right now. |
| 7 | + |
| 8 | +How this project writes prose — README, `CHANGES`, commit messages, |
| 9 | +docstrings, and source comments — is set out separately in |
| 10 | +[WRITING.md](WRITING.md). Read that before changing any of it. The |
| 11 | +constraints every change is held to, and the map of what is where, are in |
| 12 | +[AGENTS.md](../AGENTS.md). |
| 13 | + |
| 14 | +## Getting set up |
| 15 | + |
| 16 | +Development requires [uv](https://github.com/astral-sh/uv). |
| 17 | + |
| 18 | +```console |
| 19 | +$ git clone https://github.com/vcs-python/libvcs.git |
| 20 | +``` |
| 21 | + |
| 22 | +```console |
| 23 | +$ cd libvcs |
| 24 | +``` |
| 25 | + |
| 26 | +```console |
| 27 | +$ uv sync --all-extras --dev |
| 28 | +``` |
| 29 | + |
| 30 | +## The gates |
| 31 | + |
| 32 | +[ruff](https://ruff.rs) formats and lints in a single tool. The full rule |
| 33 | +set is declared in `pyproject.toml` under `[tool.ruff]`. |
| 34 | + |
| 35 | +Format: |
| 36 | + |
| 37 | +```console |
| 38 | +$ uv run ruff format . |
| 39 | +``` |
| 40 | + |
| 41 | +Lint: |
| 42 | + |
| 43 | +```console |
| 44 | +$ uv run ruff check . --fix --show-fixes |
| 45 | +``` |
| 46 | + |
| 47 | +[mypy](http://mypy-lang.org/) runs in strict mode (`[tool.mypy] strict = |
| 48 | +true`): |
| 49 | + |
| 50 | +```console |
| 51 | +$ uv run mypy . |
| 52 | +``` |
| 53 | + |
| 54 | +Test: |
| 55 | + |
| 56 | +```console |
| 57 | +$ uv run pytest |
| 58 | +``` |
| 59 | + |
| 60 | +Documentation is a gate, not a courtesy. Examples in docstrings, |
| 61 | +documentation pages under `docs/`, and `README.md` are executed by |
| 62 | +`pytest`; the doctest flags live in `pyproject.toml`, so there is no |
| 63 | +separate doctest step and a green `pytest` is the proof. Which blocks |
| 64 | +qualify, and the one mistake that silently removes a test, are in |
| 65 | +[WRITING.md](WRITING.md#documented-examples-that-run). |
| 66 | + |
| 67 | +Before claiming a test or a gate works, show it failing. A gate that has |
| 68 | +never been red is an assumption. |
| 69 | + |
| 70 | +### Imports |
| 71 | + |
| 72 | +- `from __future__ import annotations` at the top of every file. |
| 73 | +- Standard-library modules use namespace imports: `import pathlib`, not |
| 74 | + `from pathlib import Path`. Third-party packages may use |
| 75 | + `from X import Y`. |
| 76 | +- Typing: `import typing as t`, then access via namespace — |
| 77 | + `t.NamedTuple`, `t.Any`. |
| 78 | + |
| 79 | +### Logging |
| 80 | + |
| 81 | +These rules guide future logging changes; existing code may not yet |
| 82 | +conform. |
| 83 | + |
| 84 | +**Setup.** Use `logging.getLogger(__name__)` in every module. Add a |
| 85 | +`NullHandler` in library `__init__.py` files. Never configure handlers, |
| 86 | +levels, or formatters in library code — that is the application's job. |
| 87 | + |
| 88 | +**Structured context via `extra`.** Pass structured data on every log call |
| 89 | +where useful for filtering, searching, or test assertions. Core keys are |
| 90 | +stable, scalar, and safe at any log level: `vcs_cmd` (`str`, the VCS command |
| 91 | +line), `vcs_type` (`str`, git/svn/hg), `vcs_url` (`str`), `vcs_exit_code` |
| 92 | +(`int`), `vcs_repo_path` (`str`). Heavy keys — `vcs_stdout`, `vcs_stderr` |
| 93 | +(`list[str]`) — are DEBUG-only; truncate or cap them (`stdout[:100]`). |
| 94 | +Names are `snake_case` with a `vcs_` prefix. Treat established keys as |
| 95 | +compatibility-sensitive — downstream users may build dashboards and alerts |
| 96 | +on them. |
| 97 | + |
| 98 | +**Lazy formatting.** `logger.debug("msg %s", val)`, not f-strings: the |
| 99 | +interpolation is skipped entirely when the level is filtered, and a |
| 100 | +log-aggregator's message-template grouping treats `"Running %s"` as one |
| 101 | +signature instead of one per f-string value. Guard an expensive `val` with |
| 102 | +`if logger.isEnabledFor(logging.DEBUG)`. |
| 103 | + |
| 104 | +**`stacklevel` for wrappers.** Increment it for each wrapper layer so |
| 105 | +`%(filename)s:%(lineno)d` and OTel `code.filepath` point to the real |
| 106 | +caller. Verify whenever call depth changes. |
| 107 | + |
| 108 | +**`LoggerAdapter` for persistent context.** For objects with stable |
| 109 | +identity (Repository, Remote, Sync), use `LoggerAdapter` instead of |
| 110 | +repeating the same `extra` on every call. |
| 111 | + |
| 112 | +**Log levels.** `DEBUG` for internal mechanics and VCS I/O; `INFO` for |
| 113 | +repository lifecycle and user-visible operations; `WARNING` for recoverable |
| 114 | +issues, deprecations, and user-actionable config; `ERROR` for failures that |
| 115 | +stop an operation. Config-discovery noise is `DEBUG`; only a surprising or |
| 116 | +user-actionable config issue escalates to `WARNING`. |
| 117 | + |
| 118 | +**Message style.** Lowercase, past tense for events — `"repository |
| 119 | +cloned"`, `"vcs command failed"` — no trailing punctuation. Keep the |
| 120 | +message short; put details in `extra`. |
| 121 | + |
| 122 | +**Exception logging.** Use `logger.exception()` only inside an `except` |
| 123 | +block when not re-raising. Use `logger.error(..., exc_info=True)` for a |
| 124 | +traceback outside an `except` block. Avoid `logger.exception()` followed by |
| 125 | +`raise` — it duplicates the traceback. |
| 126 | + |
| 127 | +**Testing logs.** Assert on `caplog.records` attributes, not string |
| 128 | +matching on `caplog.text`: scope capture with |
| 129 | +`caplog.at_level(logging.DEBUG, logger="libvcs.cmd")`, filter records by |
| 130 | +attribute rather than position, and assert on schema |
| 131 | +(`record.vcs_exit_code == 0`, not `"exit code 0" in caplog.text`). |
| 132 | +`caplog.record_tuples` cannot access extra fields. |
| 133 | + |
| 134 | +**Avoid:** f-strings or `.format()` in log calls; unguarded logging in hot |
| 135 | +loops; catch-log-reraise without adding context; `print()` for |
| 136 | +diagnostics; logging secret env var values; non-scalar objects in `extra`; |
| 137 | +custom `extra` fields referenced in a format string without a safe default |
| 138 | +(a missing key raises `KeyError`). |
| 139 | + |
| 140 | +## Tests |
| 141 | + |
| 142 | +The suite spawns real `git`, `hg`, and `svn` processes. A test that needs a |
| 143 | +VCS binary is skipped automatically when that binary is not on `PATH` — you |
| 144 | +do not need all three installed to contribute. |
| 145 | + |
| 146 | +**Write tests as standalone functions** (`test_*`), not classes. Avoid |
| 147 | +`class TestFoo:` groupings; use descriptive function names and file |
| 148 | +organization instead. This applies to pytest tests, not doctests. |
| 149 | + |
| 150 | +**Parameterized tests** use `typing.NamedTuple` for the fixture shape: |
| 151 | + |
| 152 | +```python |
| 153 | +class RepoFixture(t.NamedTuple): |
| 154 | + test_id: str # For test naming |
| 155 | + repo_args: dict[str, t.Any] |
| 156 | + expected_result: str |
| 157 | + |
| 158 | + |
| 159 | +@pytest.mark.parametrize( |
| 160 | + list(RepoFixture._fields), |
| 161 | + REPO_FIXTURES, |
| 162 | + ids=[test.test_id for test in REPO_FIXTURES], |
| 163 | +) |
| 164 | +def test_sync(...): ... |
| 165 | +``` |
| 166 | + |
| 167 | +**Fixtures.** `src/libvcs/pytest_plugin.py` (registered as a `pytest11` |
| 168 | +entry point) provides: |
| 169 | + |
| 170 | +- `create_git_remote_repo`, `create_hg_remote_repo`, `create_svn_remote_repo` |
| 171 | + — build a temporary remote repository, each gated on its VCS binary being |
| 172 | + installed. |
| 173 | +- `git_repo`, `hg_repo`, `svn_repo` — a ready-to-use sync instance checked |
| 174 | + out from a session-cached remote; every consumer gets an isolated copy, so |
| 175 | + a test may mutate it freely, including under parallel runs. |
| 176 | +- `set_home`, `vcs_gitconfig`, `vcs_hgconfig`, `git_commit_envvars` — |
| 177 | + environment fixtures that isolate `$HOME` and VCS configuration from the |
| 178 | + host running the tests. |
| 179 | + |
| 180 | +The full reference, including the doctest-only helpers each fixture backs, |
| 181 | +is at |
| 182 | +[the pytest plugin API page](https://libvcs.git-pull.com/api/pytest-plugin/). |
| 183 | + |
| 184 | +**Running in parallel.** On a multi-core machine, [pytest-xdist](https://pytest-xdist.readthedocs.io/) |
| 185 | +spreads the real-subprocess tests across workers: |
| 186 | + |
| 187 | +```console |
| 188 | +$ just test-parallel |
| 189 | +``` |
| 190 | + |
| 191 | +This runs `uv run py.test -n auto`, where `auto` sizes the worker pool to |
| 192 | +the machine's cores. Parallelism is opt-in — `just test` and `uv run pytest` |
| 193 | +stay serial by default. |
| 194 | + |
| 195 | +**Order independence.** Tests must pass regardless of the order they run |
| 196 | +in. Keep fixtures self-contained and reset any global state in teardown. |
| 197 | +Check locally with a shuffled run: |
| 198 | + |
| 199 | +```console |
| 200 | +$ uv run --with pytest-randomly py.test -p randomly |
| 201 | +``` |
| 202 | + |
| 203 | +**Debugging.** When stuck in a debugging loop: pause and name the loop out |
| 204 | +loud, strip the reproduction down to its minimum, and write down what you |
| 205 | +tried before changing approach. Guessing repeatedly at the same fix wastes |
| 206 | +more time than the pause does. |
| 207 | + |
| 208 | +## Documentation |
| 209 | + |
| 210 | +```console |
| 211 | +$ just build-docs |
| 212 | +``` |
| 213 | + |
| 214 | +runs Sphinx and fails the build on a broken cross-reference — the doctests |
| 215 | +do not catch that, so build the docs before committing a page that adds or |
| 216 | +moves a `{ref}`, `{doc}`, or other role target. |
| 217 | + |
| 218 | +```console |
| 219 | +$ just start-docs |
| 220 | +``` |
| 221 | + |
| 222 | +starts [sphinx-autobuild](https://github.com/executablebooks/sphinx-autobuild) |
| 223 | +at <http://localhost:8068>, rebuilding on file changes. |
| 224 | + |
| 225 | +## Releasing |
| 226 | + |
| 227 | +Never create tags. Never push tags. The owner handles tagging and tag |
| 228 | +pushes, because a tag triggers the publish workflow. See |
| 229 | +[Release commits](WRITING.md#release-commits). |
| 230 | + |
| 231 | +libvcs is pre-1.0: a minor version bump (0.39 to 0.40) may contain breaking |
| 232 | +changes; a patch bump (0.39.0 to 0.39.1) is reserved for bug fixes and |
| 233 | +documentation. The version is set in `src/libvcs/__about__.py` and |
| 234 | +`pyproject.toml`. The maintainer's full release checklist is published at |
| 235 | +[Releasing](https://libvcs.git-pull.com/project/releasing/). |
| 236 | + |
| 237 | +## Pull requests |
| 238 | + |
| 239 | +One subject per pull request. Unrelated cleanup found along the way belongs |
| 240 | +in its own commit, and usually in its own pull request. |
| 241 | + |
| 242 | +Discuss a substantial change via an issue before making it. |
| 243 | + |
| 244 | +Commit format is in [WRITING.md](WRITING.md#commits). |
| 245 | + |
| 246 | +## Decorum |
| 247 | + |
| 248 | +- Participants will be tolerant of opposing views. |
| 249 | +- Participants must ensure that their language and actions are free of |
| 250 | + personal attacks and disparaging personal remarks. |
| 251 | +- When interpreting the words and actions of others, participants should |
| 252 | + always assume good intentions. |
| 253 | +- Behaviour which can be reasonably considered harassment will not be |
| 254 | + tolerated. |
| 255 | + |
| 256 | +Based on [Ruby's Community Conduct Guideline](https://www.ruby-lang.org/en/conduct/). |
| 257 | + |
| 258 | +## Security |
| 259 | + |
| 260 | +Please do not open a public issue for a vulnerability. Report it privately |
| 261 | +through the repository's |
| 262 | +[security advisories](https://github.com/vcs-python/libvcs/security). |
0 commit comments