docs(contributing): add contribution and new-container guide by totallyzen · Pull Request #460 · testcontainers/testcontainers-python · GitHub

change

Document how to contribute, with initial focus on making local development smooth.

Tasks

Finish the new-container guide
Remove any old docs referring to
Update README.md to point at the contribution guide
Update README.md to add badges (supported python versions, etc) and to give kudos to current and past maintainers and contributors

@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!

I'm using the devcontainers workflow. I could add in a section on using that if you wanted.

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 🙏

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:

make a new project on buildthedocs
"import" your fork of this repository
push changes and test there
1. significant files: /.readthedocs.yml, /Makefile (the make docs target, specifically), /conf.py (the sphinx stuff), /docs/_build (preview the result locally)
then open pr here

it might also make sense to capture what it takes to implement a new container, not just request one

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.
What about just using ruff? ruff can format code as well and is pretty fast in doing that. Then we can drop that black dependency.

I like badges 😃. So would could add the following badges:

Poetry
ruff
supported Python versions
pipeline status
test coverage Codecov, see my comment on the make file for running tests with pytest-cov

@totallyzen Can I add commits to this PR? Then I could quickly contribute my suggestions.

Can I add commits to this PR? Then I could quickly contribute my suggestions.

Yes, please do!
Including the black -> ruff transition as long as it doesn't reformat the entire repo 🙂
If we are to improve/fix formatting, I want that to be in another PR.

@bearrito actually, I think given my lack of time lately, you could contribute the devcontainers setup to this PR as well, just ping the request on this PR as well so I can track 🙏

Including the black -> ruff transition as long as it doesn't reformat the entire repo 🙂

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:

.github/workflows/main.yml to run tests, build, and publish your package when pushed to the main branch.

No such file exists.

Rebase your development branch on main (or merge main into your development branch).

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?

Add a line -e file:[feature name] to requirements.in

Where is requirements.in ??? If you add directly to pyproject.toml under a module is this sufficient and correct?

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 😀

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!

Suggested change

      
            - 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

Suggested change

      
            - 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)

Can I add commits to this PR? Then I could quickly contribute my suggestions.

@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.

Yes, please do! Including the black -> ruff transition as long as it doesn't reformat the entire repo 🙂 If we are to improve/fix formatting, I want that to be in another PR.

@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 main -> let's consider this PR to be "done"

lgtm!

…tion

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.

Apologies if that came across as brusk, if I did know anything about the desired dev setup I'd absolutely help dig in. I just cargo culted my way to a PR and just wanted to point out the draft still had some rabbit holes for someone coming cold. Is there a shaping doc anywhere of where the project is headed for dev experience?

this branch is now rebased

if someone wants to help out - let me know if you see any glaring differences between these two pages:

https://github.com/testcontainers/testcontainers-python/compare/main..docs/improve-contribution-guide.tmp

- this is the rebased version of this PR - it needs the committer reset to the author and force pushed to this branch

main...docs/improve-contribution-guide

- this is a simplified view of the files tab on this page

🤖 I have created a release *beep* *boop* --- ## [4.7.0](testcontainers-v4.6.0...testcontainers-v4.7.0) (2024-06-28) ### Features * **core:** Added Generic module ([#612](#612)) ([e575b28](e575b28)) * **core:** allow custom dockerfile path for image build and bypassing build cache ([#615](#615)) ([ead0f79](ead0f79)), closes [#610](#610) * **core:** DockerCompose.stop now stops only services that it starts (does not stop the other services) ([#620](#620)) ([e711800](e711800)) ### Bug Fixes * **ollama:** Add support for ollama module ([#618](#618)) ([5442d05](5442d05)) * **cosmosdb:** Add support for the CosmosDB Emulator ([#579](#579)) ([8045a80](8045a80)) * improve ollama docs, s/ollama_dir/ollama_home/g ([#619](#619)) ([27f2a6b](27f2a6b)) * **kafka:** Add Kraft to Kafka containers ([#611](#611)) ([762d2a2](762d2a2)) ### Documentation * **contributing:** add contribution and new-container guide ([#460](#460)) ([3519f4b](3519f4b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

type: trying

totallyzen added 3 commits March 9, 2024 16:13

chore(build): improve makefile

bc2cc40

fix: run pre-commit through poetry when calling through make

8fdff6c

docs(contributing): add CONTRIBUTING.md and draft of new-container.md

9ac3b08

totallyzen requested a review from alexanderankin March 9, 2024 15:35

bearrito reviewed Mar 9, 2024

View reviewed changes

nits

57017f9

alexanderankin force-pushed the docs/improve-contribution-guide branch 2 times, most recently from 88752d1 to 2124686 Compare March 9, 2024 22:50

alexanderankin approved these changes Mar 9, 2024

View reviewed changes

santi reviewed Mar 15, 2024

View reviewed changes

Comment thread .github/ISSUE_TEMPLATE/new-container.md Outdated

totallyzen mentioned this pull request Mar 19, 2024

Bug: tests are run inconsistently in CI pipeline #482

Closed

max-pfeiffer reviewed Mar 24, 2024

View reviewed changes

Comment thread Makefile