diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 8812438f..cd0e16a5 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -30,8 +30,10 @@ task is not relevant or leave empty when in doubt. --> - [ ] Did you read the [contributing guideline](https://github.com/omni-us/jsonargparse/blob/main/CONTRIBUTING.rst)? +- [ ] If you used a **coding agent**, did you fully understand and validate all generated code and ensure it follows the contributing guidelines? - [ ] Did you update **the documentation**? (readme and public docstrings) - [ ] Did you write **unit tests** such that there is 100% coverage on related code? (required for bug fixes and new features) - [ ] Did you verify that new and existing **tests pass locally**? +- [ ] If this is a **bug fix**, did you verify that the **tests fail without the code fix**? - [ ] Did you make sure that all changes preserve **backward compatibility**? - [ ] Did you update **the CHANGELOG** including a pull request link? (not for typos, docs, test updates, or minor internal changes/refactors) diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 9c946bb3..1c9cdd44 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -95,14 +95,31 @@ To build the documentation run: To view the built documentation, open the file ``sphinx/_build/index.html`` in a browser. +Code conventions +---------------- + +**Public vs. private naming** + +Most module filenames start with ``_``, meaning they are private implementation +details. For objects within modules, the ``_`` prefix indicates the object is +only used within that same module. An object without a ``_`` prefix may be +imported by other modules but that does not make it public — it is simply +internal to the package. The only truly public objects are those listed in +``jsonargparse.__all__`` and ``jsonargparse.typing.__all__``. + +**Type annotations and docstrings** + +New source code should be fully type annotated. Docstrings should follow `Google +style +`__. + Tests ----- Running the unit tests can be done either using `pytest `__ or `tox -`__. The tests are also installed with -the package and can be run in a non-development environment. Also pre-commit -runs some additional tests. +`__. Also pre-commit runs some additional +tests. .. code-block:: bash @@ -121,6 +138,14 @@ the main package. For example, for v4.47.0 run: pip install jsonargparse_tests==4.47.0 python -m jsonargparse_tests +All contributed features and bug fixes must include tests. For bug fixes, ensure +the test fails without the code fix. Almost always tests should exercise only +the public API. Testing internal functions directly is rarely justified and +should be avoided. For tests involving signatures, define the classes and +functions used at the global module scope. Jsonargparse is not intended to +support dynamically defined classes and functions, so there is no value in +testing such cases. + For maintainable tests: - Prefer ``pytest.mark.parametrize`` when the same test logic is exercised with @@ -131,6 +156,9 @@ For maintainable tests: harder to read. - Keep setup separate from assertions so each test clearly shows the behavior being verified. +- The pytest output must be clean. If a test causes log output, the logs must + be captured and asserted using the ``logger`` fixture and ``capture_logs`` + context manager from ``conftest.py``. Coverage @@ -188,3 +216,9 @@ should go. If you are unsure, ask in the pull request. Please don't open pull requests with breaking changes unless this has been discussed and agreed upon in an issue. + +Contributions using coding agents are welcome. However, any agent-generated +code must be fully understood by the submitter and must make sense and follow +these contributing guidelines. Always ask the agent to read and follow these +guidelines. Also ask to read ``.github/PULL_REQUEST_TEMPLATE.md`` so that the +tasks before submitting are covered.