docs(contributing): add contribution and new-container guide #460
Conversation
|
@jankatins @bearrito @max-pfeiffer your comments are very welcome - It's still in draft, but it's a start! This is what I had time for today, but will continue on the weekday evenings! |
|
@santi also, this will be useful for you! |
| - `pyenv` **Recommended**: For installing python versions for your system. | ||
| Poetry infers the current latest version from what it can find on the `PATH` so you are still fine if you don't use `pyenv`. | ||
|
|
||
| ### Build and test |
There was a problem hiding this comment.
I'm using the devcontainers workflow. I could add in a section on using that if you wanted.
There was a problem hiding this comment.
hey @bearrito apologies for the delays, I'm finding myself with some time today.
I'd love your contribution on the devcontainers workflow, but lemme finish this PR (ETA today, finally found the time to work on this) and then yo can raise your own PR for that addition to the docs 🙏
alexanderankin
force-pushed
the
docs/improve-contribution-guide
branch
2 times, most recently
from
88752d1
to
2124686
Compare
|
looks good to me, i find this stuff very valuable but right now i dont really know what to add as far as suggestions or discussion. my head is mostly in the outstanding PRs and releases and such. also sorry for the double rebase. i did a rebase locally onto main. then i found a button on github, but the button on github gave me credit for all your work, so then i just pushed my local rebase. |
|
it might also make sense to capture what it takes to implement a new container, not just request one |
|
quick summary of how to test buildthedocs changes:
|
Yes, I think so too. We could briefly elaborate about the best practices that we want to see in contributions. So modules are a bit more consistent. Maybe also what to include in README.rst in the modules directory. To me it's not clear what purpose that serves currently. |
|
I would say also our .pre-commit-config.yaml also needs some attention: there we have black for code formatting and ruff as linter defined. I like badges 😃. So would could add the following badges:
@totallyzen Can I add commits to this PR? Then I could quickly contribute my suggestions. |
Yes, please do! @bearrito actually, I think given my lack of time lately, you could contribute the |
Ruff is compatible with black apart from a very few and rare differences. I've several repos here where black and ruff format are interchangeable :-) |
... based on @jankatins Co-authored-by: Jan Katins <jan.katins@katzien.de>
|
@totallyzen I can do that. It will either be today or tomorrow. Likely tomorrow. |
|
@totallyzen Can I get this approved if it looks okay- #506 Namely I want to be able to reference that work in the docs for devcontainers |
|
As someone who just tried to raise a PR with a new container, and just checked out these new docs. Can I point out its not obvious at all on these points:
No such file exists.
Main just changed whilst I've had my PR up, is the preference I rebase and force push over the top of my already published PR constantly?
Where is |
|
Hey @MattOates, Thanks for the pointers, I'm sorry for your bad experience, but please note that this PR is a draft, I did not have time to finish it. A lot of people left comments on it, all duly noted, you can in fact raise a PR against this branch and make suggestions. 👍 |
|
Hey @MattOates sorry for your rough start here, but we really appreciate you contributions 😊 Hopefully the latest additions to the draft should make clear the things you are wondering about. We have just revamped the whole structure and development setup in this repo, so no wonder the docs are very outdated. I hope you stay and find the new setup easier to work with 😀 |
There was a problem hiding this comment.
A few last comments from my end, but I think the current draft is in a good shape and ready to be released. Further contributions are of course welcome and can be added in future PRs.
Great job here @totallyzen!
| for the given tools you are triyng to implement. | ||
| - This means we want you to avoid adding specific libraries as dependencies to `testcontainers`. | ||
| - Therefore, you should implement the configuration and the waiting with as little extra as possible | ||
| - You may still find it useful to add your preferred client library as a dev dependency |
There was a problem hiding this comment.
| - You may still find it useful to add your preferred client library as a dev dependency | |
| - If the intended usage of the new Testcontainer is with a client library, please add the client library as a dev dependency, so that tests can mirror actual usage of the new container. |
not sure if this is the best wording but maybe we encourage people to add dev dependencies instead of writing tests that never run
There was a problem hiding this comment.
| - You may still find it useful to add your preferred client library as a dev dependency | |
| - If the intended usage of the new Testcontainer is with a client library, please add such a client library as a dev dependency and a test which mirrors actual usage of the container.` |
(e.g. PG has multiple which consume the url)
@totallyzen I just created this PR containing some badges: #528 I also picked up communications with @kiview on Slack to make code coverage reports and that codecov.io badge happen. |
@totallyzen I just created a PR and removed that black dependency: #529 Just a couple of files became re-formatted. So this we have rather little code changes. |
I added the following badges: - Poetry - Ruff - Supported Python versions - Workflow runs
|
I'll merge this, I heard no negative comments on the existing additions and some improvements rely on |
|
lgtm! |
change
Document how to contribute, with initial focus on making local development smooth.
Tasks
new-containerguideREADME.mdto point at the contribution guideREADME.mdto add badges (supported python versions, etc) and to give kudos to current and past maintainers and contributors