Update Contribution types, Reference, and In depth explanations in Contributing guide#2521
Conversation
| [TensorFlow's style guide](https://www.tensorflow.org/community/contribute/code_style) | ||
| or the [Google style guide](https://github.com/google/styleguide/blob/gh-pages/pyguide.md). | ||
| Both more or less follow PEP 8. | ||
| TODO: Details and sections to be added... |
There was a problem hiding this comment.
Just flagging that I'm not sure what are the sections to add here.
Also, should "## Docstring formatting", "## Documentation for user facing methods", and "## Tests" be included here?
There was a problem hiding this comment.
I would mention the workflow is forking, developing locally, running checks (tox mention, see later) then opening a PR, plus a link to the PR step-by-step tutorial for anyone who might be new.
Second paragraph probably a quick overview about using tox to help with devops tasks, plus a mention of tox list -m dev to see the available tasks.
I do think all these 3 sections should instead be subsections inside this one.
| We prefer that issues be filled on the | ||
| Github Issue Tracker of the corresponding package repository | ||
| ([`arviz-base`](https://github.com/arviz-devs/arviz-base/issues), | ||
| [`arviz-stats`](https://github.com/arviz-devs/arviz-stats/issues), | ||
| [`arviz-plots`](https://github.com/arviz-devs/arviz-stats/issues)) |
There was a problem hiding this comment.
I am not completely sure about that. Given we are on github and transfering issues is clicking a button, I am kind of leaning towards directing most people to the main repo and we can move them later on. Otherwise I fear it will be added friction, especially for things that don't fit completely into a single repo.
Opening issues on the respective repos is also perfectly fine and not discouraged at all though. Not sure how to communicate this, maybe focus on the "whichever is easier for you as the reporter" part
There was a problem hiding this comment.
I just wrote it in a different way so it says both things
| * Please prefix the title of incomplete contributions with `[WIP]` (to indicate a work in progress). | ||
| WIPs may be useful to (1) indicate you are working on something to avoid duplicated work, | ||
| (2) request a broad review of functionality or API, or (3) seek collaborators. |
There was a problem hiding this comment.
I think we have mostly shifted towards using draft PRs ourselves. The reasoning stays the same, the way to signal it changed a bit
| $ pip install black | ||
| $ black arviz/ examples/ asv_benchmarks/ | ||
| ```shell | ||
| tox -e py312-coverage |
There was a problem hiding this comment.
maybe comment # or full-coverage, nightlies... to make sure it is clear each repo may have their own test running commands.
Having said that I don't really care much either way what we put here but figured I'd mention. I never run the -coverage version tests locally. I don't think it adds too much overhead (but obviously haven't tested it) but I don't know the current coverage so I would have to go check codecov to do anything with the output. If I have to go to codecov I just wait for the PR and get the full report there which is nicer.
There was a problem hiding this comment.
If I have to go to codecov I just wait for the PR and get the full report there which is nicer.
Yes, I do the same. So let's reflect that in the docs.
Co-authored-by: Oriol Abril-Pla <oriol.abril.pla@gmail.com>
Co-authored-by: Oriol Abril-Pla <oriol.abril.pla@gmail.com>
Co-authored-by: Oriol Abril-Pla <oriol.abril.pla@gmail.com>
Co-authored-by: Oriol Abril-Pla <oriol.abril.pla@gmail.com>
OriolAbril
left a comment
OriolAbril
left a comment
There was a problem hiding this comment.
let's merge and continue working if needed
2 participants
Uh oh!
There was an error while loading. Please reload this page.