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

Add contributing guidelines #367

Merged
merged 6 commits into from
Oct 10, 2023
Merged

Add contributing guidelines #367

merged 6 commits into from
Oct 10, 2023

Conversation

pjbull
Copy link
Member

@pjbull pjbull commented Oct 8, 2023

First pass at contributing guidelines and a code of conduct in case folks from hacktoberfest come through.

Closes #213

@pjbull pjbull requested a review from jayqi October 8, 2023 20:52
@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2023

@github-actions github-actions bot temporarily deployed to pull request October 8, 2023 20:53 Inactive
@codecov
Copy link

codecov bot commented Oct 8, 2023

Codecov Report

Merging #367 (015d4c7) into master (703607c) will decrease coverage by 0.9%.
The diff coverage is n/a.

❗ Current head 015d4c7 differs from pull request most recent head 2c9724c. Consider uploading reports for the commit 2c9724c to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #367     +/-   ##
========================================
- Coverage    93.7%   92.8%   -0.9%     
========================================
  Files          22      22             
  Lines        1563    1563             
========================================
- Hits         1465    1452     -13     
- Misses         98     111     +13     

see 3 files with indirect coverage changes

Copy link
Contributor

@klwetstone klwetstone left a comment

Choose a reason for hiding this comment

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

@pjbull These are very clear and extremely well written! Easy to read, includes all the details needed, and with exactly the right level of detail. My comments are generally about page flow and organization.

I'd recommend making a clearer distinction between key steps needed for any PR vs. general tips about working the repo, each of which which may only be relevant to some PRs. I would add a section at the top (right under the bulleted "first, a few guidelines") that's something like:

How to contribute

  1. File an issue. 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 before getting started.
  2. Create a local environment.
  3. Make changes in your local version of the repository.
  4. Make sure that the tests pass locally.
  5. Update the package documentation, if applicable.
  6. Go through the items in the final PR checklist.
  7. Submit 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.

The content is GREAT -- comprehensive, easy to understand, and thorough. To make it easier to find the right pieces of info, I would reorganize the sections to match the order of the new "how to contribute" bullets, and to separate out steps that are always necessary from tips/tricks that may not be relevant to every PR. My suggested outline would be:

## Local development
- Just instructions to create an environment - what's currently under "local development but before "adding dependencies"
## Tests
- Same sections as before, minus interactive testing
- Change each sub-header to be "###" instead of "####"
## Documentation
- Same sections as before
- Change each sub-header to be "###" instead of "####"
## Submitting a PR (same sections as before)
## Code standards and tips
  ### Adding dependencies
  ### Linting and formatting
  ### Type hinting
  ### Credentials and cloud access
  ### Interactive testing (moved from the section on "tests")
  ### Exceptions (moved from "code architecture")
## Code architecture (same sections as before minus exceptions)
## Governance

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to pull request October 9, 2023 19:56 Inactive
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
pjbull and others added 4 commits October 9, 2023 14:32
Co-authored-by: Katie Wetstone <46792169+klwetstone@users.noreply.github.com>
@github-actions github-actions bot temporarily deployed to pull request October 10, 2023 00:06 Inactive
@pjbull pjbull merged commit 6149544 into master Oct 10, 2023
1 of 6 checks passed
@pjbull pjbull deleted the contrib branch October 10, 2023 00:06
msmitherdc added a commit to CRREL/cloudpathlib that referenced this pull request Nov 1, 2023
* Add "CloudPath" to possible cloud_path types (drivendataorg#342) (drivendataorg#343)

* Add "CloudPath" to possible cloud_path types (drivendataorg#342)

* Add to changelog

---------

Co-authored-by: Karol Zlot <11590960+karolzlot@users.noreply.github.com>

* Added with_stem support (drivendataorg#290) (drivendataorg#354)

* Added with_stem support

* Update readme (+small fix to make_support_table)

* Added test

* Update history.md

* Fallback to trivial CPython implementation if `with_stem` is not available

* Add a clarification comment

Co-authored-by: Daniel Oriyan <gilthans@gmail.com>

* Fix flake8 errors (drivendataorg#359)

* Fix flake8 errors

* Fix wrong comparison

* Bump codecov action version; add codecov token

---------

Co-authored-by: Jay Qi <jayqi@users.noreply.github.com>

* Update write_text to include newline variable (drivendataorg#362) (drivendataorg#363)

* Update write_text to include newline variable

* Remove old docstring link

* Update cloudpathlib/cloudpath.py with black correction

* Remove use of io module

* update HISTORY.md

Co-authored-by: Matthew Price <matthew.price23@gmail.com>

* Add support for Python 3.12 (drivendataorg#364)

* passing tests

* remove print

* make method signatures match

* Remove unused import

* ignore type errors

* ignore more type errors

* make linting and tests work on multiple py versions

* add 3.12 to CI and pyproject

* use pytest-cases fork

* More performant walk implementation

* format

* update methods

* Test walk method

* Version agnostic tests

* update tests

* Add tests

* Order agnostic walk test

* Changes

* Update changelog

* sleep for flaky test

* Sleep flaky tests

* sleepier

* implement env var for local cache dir (drivendataorg#361)

* Add contributing guidelines (drivendataorg#367)

* contributing guidelines

* Apply suggestions from code review

Co-authored-by: Katie Wetstone <46792169+klwetstone@users.noreply.github.com>

* WIP

* Contrib tweaks and PR template

* clarification

* update history typo

---------

Co-authored-by: Katie Wetstone <46792169+klwetstone@users.noreply.github.com>

* Version bump

* format with black

* ignore type checking here

* update client reference

* update

* no need to change

---------

Co-authored-by: Peter Bull <pjbull@gmail.com>
Co-authored-by: Karol Zlot <11590960+karolzlot@users.noreply.github.com>
Co-authored-by: Daniel Oriyan <gilthans@gmail.com>
Co-authored-by: Jay Qi <2721979+jayqi@users.noreply.github.com>
Co-authored-by: Jay Qi <jayqi@users.noreply.github.com>
Co-authored-by: Matthew Price <matthew.price23@gmail.com>
Co-authored-by: Peter Bull <peter@drivendata.org>
Co-authored-by: Katie Wetstone <46792169+klwetstone@users.noreply.github.com>
Co-authored-by: Michael D. Smith <michael.smith@erdc.dren.mil>
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.

No Contributing Guidelines for the Project
3 participants