-
Notifications
You must be signed in to change notification settings - Fork 85
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
Refactor development guide #874
base: main
Are you sure you want to change the base?
Conversation
This should: * provide a note that our counda env name has changed * provide some guidance on which style of dev env (or not) to use * provide some guidance on working in an IDE * better explains how to use nox
docs/contributing/development.md
Outdated
!!! info "Important" | ||
|
||
Currently, our integration tests are *flakey* and a small number of random failures are expected. When the integration | ||
test suite runs, it may retun a status code of 99 if the failure rate was less than an "acceptable" threshold. Since | ||
any non-zero status code is considered an error, your console and/or IDE wll consider this a failure by default. | ||
`nox`, however, knows about this special status code and will report a success. To get pytest or your IDE to match | ||
this behavior, you can modify the special status code to be zero with the `EARTHACCESS_ALLOWABLE_FAILURE_STATUS_CODE` | ||
evnironment variable: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some discussion related to this here:
#872 (comment)
Also, I opened a related discussion here about what we install into our development environments by default: |
@jhkennedy I think these changes help a lot. A few suggestions. In your comment above you say
I wonder if we should add a note to the Also, should we add a section between the "Development Environment Setup" and "Running Tests" about development workflow and branching? I don't think we can push to |
This PR is largely prompted by recent conversations in our hackweeks, #858, and some discussions in the Openscapes slack (e.g., https://openscapes.slack.com/archives/C05TMK269HA/p1730312039229589).
Basically:
This PR should:
I think, overall, this should make things more approachable for contributors, but I suspect this proposal may generate some discussion, so it'd be good to get a few reviewer's eyes on this.
I'm happy to make changes -- this is just a starting point IMO.
Note: I do lean heavily into mkdocs syntax, so you may want to view the preview linked below or checkout this branch and build the docs locally while reviewing this PR.
Pull Request (PR) draft checklist - click to expand
contributing documentation
before getting started.
title such as "Add testing details to the contributor section of the README".
Example PRs: #763
example
closes #1
. SeeGitHub docs - Linking a pull request to an issue.
CHANGELOG.md
with details about your change in a section titled## Unreleased
. If such a section does not exist, please create one. FollowCommon Changelog for your additions.
Example PRs: #763
README.md
with details of changes to theearthaccess interface, if any. Consider new environment variables, function names,
decorators, etc.
Click the "Ready for review" button at the bottom of the "Conversation" tab in GitHub
once these requirements are fulfilled. Don't worry if you see any test failures in
GitHub at this point!
Pull Request (PR) merge checklist - click to expand
Please do your best to complete these requirements! If you need help with any of these
requirements, you can ping the
@nsidc/earthaccess-support
team in a comment and wewill help you out!
Request containing "pre-commit.ci autofix" to automate this.
📚 Documentation preview 📚: https://earthaccess--874.org.readthedocs.build/en/874/