Contrib tweaks and PR template

drivendataorg · Oct 10, 2023 · 015d4c7 · 015d4c7
1 parent ae987ff
commit 015d4c7
Show file tree

Hide file tree

Showing 4 changed files with 137 additions and 117 deletions.
diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
@@ -0,0 +1,18 @@
+DESCRIPTION_HERE
+
+Closes #ISSUE
+
+----------------
+
+Contributor checklist:
+
+ - [ ] I have read and understood `CONTRIBUTING.md`
+ - [ ] Confirmed an issue exists for the PR, and the text `Closes #issue` appears in the PR summary (e.g., `Closes #123`).
+ - [ ] Confirmed PR is rebased onto the latest base
+ - [ ] Confirmed failure before change and success after change
+ - [ ] Any generic new functionality is replicated across cloud providers if necessary
+ - [ ] Tested manually against live server backend for at least one provider
+ - [ ] Added tests for any new functionality
+ - [ ] Linting passes locally
+ - [ ] Tests pass locally
+ - [ ] Updated `HISTORY.md` with the issue that is addressed and the PR you are submitting. If the top section is not `## UNRELEASED``, then you need to add a new section to the top of the document for your change.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -5,21 +5,21 @@ Thanks for offering to help on `cloudpathlib`! We welcome contributions. This do
 First, a few guidelines:
 
  - Follow the [code of conduct](CODE_OF_CONDUCT.md).
- - No PRs from outside contributors are accepted without an issue. Please file an issue if you think a change is necessary. This is so you don't waste your time, so please get maintainer sign off before you work.
+ - PRs from outside contributors will not be accepted without an issue. We respect your time and want to make sure any work you do will be reviewed, so please wait for a maintainer to sign off on the issue before getting started. 
  - If you are looking just to make a contribution, look at issues with [label "good first issue"](https://github.com/drivendataorg/cloudpathlib/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22).
 
 
 ## How to contribute
 
-0. File an [issue](https://github.com/drivendataorg/cloudpathlib/issues). No PRs from outside contributors are accepted without an issue. We respect your time and want to make sure any work you do will be reviewed, so please wait for a maintainer to sign off on the issue before getting started. 
+0. As noted above, please file an [issue](https://github.com/drivendataorg/cloudpathlib/issues) if you are not fixing an existing issue.
 1. Fork the repo, clone it locally, and create a [local environment](#local-development).
 2. Make changes in your local version of the repository.
 3. Make sure that the [tests](#tests) pass locally.
 4. Update the package [documentation](#documentation), if applicable.
 5. Go through the items in the final [PR checklist](#pr-checklist).
 6. [Submit a PR](#submitting-a-pr)
 
-For some guidance on working with the code, see the sections on [code standards](link to new section described below) and [code architecture](#code-architecture).
+For some guidance on working with the code, see the sections on [code standards](#code-standards-and-tips) and [code architecture](#code-architecture).
 
 
 ## Local development
@@ -57,42 +57,9 @@ make reqs
 
 This will also install an editable version of `cloudpathlib` with all the extras into your environment.
 
-### Adding dependencies
-
-We want `cloudpathlib` to be as lightweight as possible. Our strong preference is to not take any external dependencies for the library outside of the official software development kit (SDK) for the cloud provider. If you want to add a dependency, please open an issue to discuss it first. Library depencies are tracked in `pyproject.toml`.
-
-Dependencies that are only needed for building documentation, development, linting, formatting, or testing can be added to `requirements-dev.txt`, and are not subject to the same scrutiny.
-
-
-### Linting and formatting
-
-Any code changes need to follow the code standards of the project. We use `black` for formatting code (as configured in `pyproject.toml`), and `flake8` for linting (as configured in `setup.cfg`).
-
-To apply these styles to your code, you can run:
-
-```bash
-make format
-```
+## Tests
 
-To ensure that your code is properly formatted and linted and passes type checking, you can run:
-
-```bash
-make lint
-```
-
-### Type hinting
-
-Any public APIs need to have proper type annotations, which are checked by `mypy` (as configured in `pyproject.toml`) when you run the `make lint` command. These need to pass. If you are adding a new public API, you will need to add type annotations to it. If you are changing an existing public API, you will need to update the type annotations to match the new API. If you are adding a private API, you do not need to add type annotations, but you should consider it. Only use `# type: ignore` or `Any` if there is not other way.
-
-As mentioned, to ensure your code passes `mypy` type checking, you can run:
-
-```bash
-make lint
-```
-
-### Tests
-
-#### Commands
+### Commands
 
 There is a robust test suite that covers most of the core functionality of the library. There are a few different testing commands that are useful to developers.
 
@@ -114,7 +81,7 @@ Finally, you may want to run your tests against live servers to ensure that the
 make test-live-cloud
 ```
 
-#### Test rigs
+### Test rigs
 
 Since we want behavior parity across providers, nearly all of the tests are written in a provider-agnositc way. Each test is passed a test rig as a fixture, and the rig provides the correct way for generating cloudpaths for testing. The test rigs are defined in [`conftest.py`](tests/conftest.py).
 
@@ -127,9 +94,9 @@ When a test suite runs against the rig, the rig does the following steps on setu
 
 When the tests finish, if it is using a live server, the test files will be deleted from the provider.
 
-If you want to speed up your testing, you may comment out some of the rigs in [`conftest.py`](tests/conftest.py) during development. Don't commit this change, and make sure you run against all the rigs before submitting a PR.
+If you want to speed up your testing during development, you may comment out some of the rigs in [`conftest.py`](tests/conftest.py). Don't commit this change, and make sure you run against all the rigs before submitting a PR.
 
-#### Authoring tests
+### Authoring tests
 
 We want our test suite coverage to be comprehensive, so PRs need to add tests if they add new functionality. If you are adding a new feature, you will need to add tests for it. If you are changing an existing feature, you will need to update the tests to match the new behavior.
 
@@ -153,6 +120,111 @@ If you are testing functionality on the `*Client` class, you can get an instance
 new_client = rig.client_class()
 ```
 
+## Documentation
+
+We also aim to have robust and comprehensive documentation. For public API functions, we provide docstrings that explain how things work to end users, and these are automatically built into the documentation. For more complex topics, we write specific documentation.
+
+We use [mkdocs](https://www.mkdocs.org/) to generate the documentation.
+
+
+### Building
+
+To build the latest version of the documentation, you can run:
+
+```bash
+make docs
+```
+
+### Serving
+
+While you are developing, you can serve a local version of the docs to see what your changes look like. This will auto-reload for most changes to the docs:
+
+```bash
+make docs-serve
+```
+
+Note that the main page (`index.md`), the changelog (`HISTORY.md`), and the contributing page (`CONTRIBUTING.md`) are all generated from the files in the project root. If you want to make changes to the documentation, you should make them in the root of the project and then run `make docs-setup` to update the other files. **The dev server does not automatically pick up these changes.** You will need to stop the server and restart it to see the changes.
+
+### Authoring
+
+Documentation pages can either be authored in normal Markdown or in a runnable jupyter notebook. Notebooks are useful for showing examples of how to use the library. You can see an example of a notebook in [`docs/docs/why_cloudpathlib.ipynb`](docs/docs/why_cloudpathlib.ipynb).
+
+Note: generating the documentation **does not** execute any notebooks, it just converts them. You need to restart and run all notebook cells to make sure the notebook executes top-to-bottom and has the latest results before committing it.
+
+### Docstrings
+
+For public APIs, please add a docstring that will appear in the documentation automatically. Since public APIs are type-hinted, there is no need to list the function parameters, their types, and the return types in a specific format in the docstring. Instead, you should describe what the function does and any relevant information for the user.
+
+## Submitting a PR
+
+Once you have everything working as expected locally, submit a PR. The PR will be automatically tested by GitHub Actions. If the tests fail, you will need to fix the issue and push a new commit to the PR. If the tests pass (except the live tests, as noted below), you will need to get a maintainer to review the PR and merge it.
+
+### PR checklist
+
+Here's a checklist from the PR template to make sure that you did all the required steps:
+
+```
+ - [ ] I have read and understood `CONTRIBUTING.md`
+ - [ ] Confirmed an issue exists for the PR, and the text `Closes #issue` appears in the PR summary (e.g., `Closes #123`).
+ - [ ] Confirmed PR is rebased onto the latest base
+ - [ ] Confirmed failure before change and success after change
+ - [ ] Any generic new functionality is replicated across cloud providers if necessary
+ - [ ] Tested manually against live server backend for at least one provider
+ - [ ] Added tests for any new functionality
+ - [ ] Linting passes locally
+ - [ ] Tests pass locally
+ - [ ] Updated `HISTORY.md` with the issue that is addressed and the PR you are submitting. If the top section is not `## UNRELEASED``, then you need to add a new section to the top of the document for your change.
+```
+
+### PR CI/CD test run
+
+If you are not a maintainer, a maintainer will have to approve your PR to run the test suite in GitHub Actions. No need to ping a maintainer, it will be seen as part of our regular review.
+
+Even once the tests run, two jobs will fail. This is expected. The failures are: (1) The live tests, and (2) the install tests. Both of these require access to the live backends, which are not available to outside contributors. If everything else passes, you can ignore these failiures. A mainter will take the following steps:
+
+ - Create a branch off the main repo for your PR's changes
+ - Merge your PR into that new branch
+ - Run CI/CD on the repo-local branch which has access to the live backends
+ - Confirm the live tests pass as expected. (If not, you will need to fix the issue and create another PR into this reopo-local branch.)
+ - Once they pass, merge the repo-local branch into the main branch.
+
+For example, see a [repo-local branch running the live tests in this PR](https://github.com/drivendataorg/cloudpathlib/pull/354).
+
+## Code standards and tips
+
+### Adding dependencies
+
+We want `cloudpathlib` to be as lightweight as possible. Our strong preference is to not take any external dependencies for the library outside of the official software development kit (SDK) for the cloud provider. If you want to add a dependency, please open an issue to discuss it first. Library depencies are tracked in `pyproject.toml`.
+
+Dependencies that are only needed for building documentation, development, linting, formatting, or testing can be added to `requirements-dev.txt`, and are not subject to the same scrutiny.
+
+
+### Linting and formatting
+
+Any code changes need to follow the code standards of the project. We use `black` for formatting code (as configured in `pyproject.toml`), and `flake8` for linting (as configured in `setup.cfg`).
+
+To apply these styles to your code, you can run:
+
+```bash
+make format
+```
+
+To ensure that your code is properly formatted and linted and passes type checking, you can run:
+
+```bash
+make lint
+```
+
+### Type hinting
+
+Any public APIs need to have proper type annotations, which are checked by `mypy` (as configured in `pyproject.toml`) when you run the `make lint` command. These need to pass. If you are adding a new public API, you will need to add type annotations to it. If you are changing an existing public API, you will need to update the type annotations to match the new API. If you are adding a private API, you do not need to add type annotations, but you should consider it. Only use `# type: ignore` or `Any` if there is not other way.
+
+As mentioned, to ensure your code passes `mypy` type checking, you can run:
+
+```bash
+make lint
+```
+
 #### Interactive testing
 
 To interactively test the library, we recommend creating a Jupyter notebook in the root of the project called `sandbox.ipynb`. We `.gitignore` a `sandbox.ipynb` file by default for this purpose. You can import the library and run commands in the notebook to test functionality. This is useful for testing new features or debugging issues.
@@ -172,7 +244,6 @@ from cloudpathlib import CloudPath
 cp = CloudPath("s3://my-test-bucket/")
 ```
 
-
 ### Credentials and cloud access
 
 For certain tests and development scenarios, you will need to have access to the relevant cloud provider. You can put the authentication credentials into a `.env` file in the root of the project. The `.env` file is ignored by git, so you can put your credentials there without worrying about them being committed. See the [authentication documentation](https://cloudpathlib.drivendata.org/stable/authentication/) for information on what variables to set.
@@ -218,78 +289,10 @@ These can be run with `make perf`. This will generate a report like:
 
 To see how it is used in PR, you can [see an example here](https://github.com/drivendataorg/cloudpathlib/pull/364).
 
+### Exceptions
 
-### Documentation
-
-We also aim to have robust and comprehensive documentation. For public API functions, we provide docstrings that explain how things work to end users, and these are automatically built into the documentation. For more complex topics, we write specific documentation.
-
-We use [mkdocs](https://www.mkdocs.org/) to generate the documentation.
-
-
-#### Building
-
-To build the latest version of the documentation, you can run:
-
-```bash
-make docs
-```
-
-#### Serving
-
-While you are developing, you can serve a local version of the docs to see what your changes look like. This will auto-reload for most changes to the docs:
-
-```bash
-make docs-serve
-```
-
-Note that the main page (`index.md`), the changelog (`HISTORY.md`), and the contributing page (`CONTRIBUTING.md`) are all generated from the files in the project root. If you want to make changes to the documentation, you should make them in the root of the project and then run `make docs-setup` to update the other files. **The dev server does not automatically pick up these changes.** You will need to stop the server and restart it to see the changes.
-
-#### Authoring
-
-Documentation pages can either be authored in normal Markdown or in a runnable jupyter notebook. Notebooks are useful for showing examples of how to use the library. You can see an example of a notebook in [`docs/docs/why_cloudpathlib.ipynb`](docs/docs/why_cloudpathlib.ipynb).
-
-Note: generating the documentation **does not** execute any notebooks, it just converts them. You need to restart and run all notebook cells to make sure the notebook executes top-to-bottom and has the latest results before committing it.
-
-#### Docstrings
-
-For public APIs, please add a docstring that will appear in the documentation automatically. Since public APIs are type-hinted, there is no need to list the function parameters, their types, and the return types in a specific format in the docstring. Instead, you should describe what the function does and any relevant information for the user.
-
-
-## Submitting a PR
-
-Once you have everything working as expected locally, submit a PR. The PR will be automatically tested by GitHub Actions. If the tests fail, you will need to fix the issue and push a new commit to the PR. If the tests pass (except the live tests, as noted below), you will need to get a maintainer to review the PR and merge it.
-
-### PR checklist
-
-Here's a checklist you can put in your PR to make sure that you did all the required steps:
-
-
-```
- - [ ] I have read and understood `CONTRIBUTING.md`
- - [ ] Confirmed an issue exists for the PR, and the text `Closes #issue` appears in the PR summary (e.g., `Closes #123`).
- - [ ] Confirmed PR is rebased onto the latest base
- - [ ] Confirmed failure before change and success after change
- - [ ] Any generic new functionality is replicated across cloud providers if necessary
- - [ ] Tested manually against live server backend for at least one provider
- - [ ] Added tests for any new functionality
- - [ ] Linting passes locally
- - [ ] Tests pass locally
- - [ ] Updated `HISTORY.md` with the issue that is addressed and the PR you are submitting. If the top section is not `## UNRELEASED``, then you need to add a new section to the top of the document for your change.
-```
-
-### PR CI/CD test run
-
-If you are not a maintainer, a maintainer will have to approve your PR to run the test suite in GitHub Actions. No need to ping a maintainer, it will be seen as part of our regular review.
-
-Even once the tests run, two jobs will fail. This is expected. The failures are: (1) The live tests, and (2) the install tests. Both of these require access to the live backends, which are not available to outside contributors. If everything else passes, you can ignore these failiures. A mainter will take the following steps:
-
- - Create a branch off the main repo for your PR's changes
- - Merge your PR into that new branch
- - Run CI/CD on the repo-local branch which has access to the live backends
- - Confirm the live tests pass as expected. (If not, you will need to fix the issue and create another PR into this reopo-local branch.)
- - Once they pass, merge the repo-local branch into the main branch.
+Different backends may raise different exception classses when something goes wrong. To make it easy for users to catch exceptions that are agnostic of the backend, we generally will catch and raise a specific exception from [`exceptions.py`](cloudpathlib/exceptions.py) for any exception that we understand. You can add new exceptions to this file if any are needed for new features.
 
-For example, see a [repo-local branch running the live tests in this PR](https://github.com/drivendataorg/cloudpathlib/pull/354).
 
 
 ## Code architecture
@@ -317,10 +320,6 @@ Any code that needs to interact with the provider does so by calling methods on
 
 Some methods are implemented on the `*Path` class for the specific provider. This is reserved for two cases: (1) provider-specific properties, like `S3Path.bucket` or `AzureBlobPath.container`, and (2) methods that are more efficiently implemented in a provider-specific way, like `S3Path.stat`. 
 
-### Exceptions
-
-Different backends may raise different exception classses when something goes wrong. To make it easy for users to catch exceptions that are agnostic of the backend, we generally will catch and raise a specific exception from [`exceptions.py`](cloudpathlib/exceptions.py) for any exception that we understand. You can add new exceptions to this file if any are needed for new features.
-
 ### Adding a new provider
 
 Adding a new provider is relatively straightforward. If you are extending `cloudpathlib`, but don't intend to make your provider part of the core library, implement the following pieces.

diff --git a/HISTORY.md b/HISTORY.md
@@ -6,6 +6,7 @@
  - Add `newline` parameter to the `write_text` method to align to `pathlib` functionality as of Python 3.10. [PR #362]https://github.com/drivendataorg/cloudpathlib/pull/362), thanks to [@pricemg](https://github.com/pricemg).
  - Add support for Python 3.12 ([PR #364](https://github.com/drivendataorg/cloudpathlib/pull/364))
  - Add `CLOUDPATHLIB_LOCAL_CACHE_DIR` env var for setting local_cache_dir default for clients ([Issue #352](https://github.com/drivendataorg/cloudpathlib/issues/352), [PR #357](https://github.com/drivendataorg/cloudpathlib/pull/357))
+ - Add `CONTRIBUTING.md` instructions for contributors ([Issue #213](https://github.com/drivendataorg/cloudpathlib/issues/213), [PR #367](https://github.com/drivendataorg/cloudpathlib/pull/367))
 
 ## v0.15.1 (2023-07-12)