Add contributing docs

[collerek]

Allows testing all backends on local using docker.

Added contributing section in docs.

[aminalaee]

Nice work, I guess it'd be nice to split this into multiple PRs so reviewing it can be easier.

[collerek]

Nice work, I guess it'd be nice to split this into multiple PRs so reviewing it can be easier.

You want to split adding docker and adding windows compatibility in separate PRs?

[aminalaee]

yeah and contributing docs too.

[collerek]

But contributing already includes a section about running tests with docker, so you want me to create a separate version that will be overwritten by the next pr? 😛

[collerek]

OK now it's only docker support and contributing

[collerek]

@aminalaee Any chance for a review of those 2 (plus #454 )?
The changes are quite simple and will allow local development without relying on CI workflow alone.

[aminalaee]

To be honest I've taken a look but I'm not sure if it's the best approach. Sorry I appreciate the effort but I wouldn't like that. You can also wait for someone else maybe they do have a different opinion here.

I would personally leave docker out of this and only do the contributing docs here. I'll put the comment for the other one there.

[collerek]

To be honest I've taken a look but I'm not sure if it's the best approach. Sorry I appreciate the effort but I wouldn't like that. You can also wait for someone else maybe they do have a different opinion here.

I would personally leave docker out of this and only do the contributing docs here. I'll put the comment for the other one there.

@aminalaee
This is in reality the only way to provide a test isolation environment on a local machine.

The alternative is a need to install all databases servers on your local machine but that post several problems, among others:

• what if you want to test different server versions (now you either install/uninstall or have multiple servers of a type on the local machine (if allowed))
• servers are not cleaned on their own - sure you can roll back transactions, delete data etc. but you leave an opportunity that you mess something up and you will leave some data in the database. That, in turn, might cause your test to either fail or pass even if they shouldn't
• having a local server as a machine vise solution might discourage developers if they already have one, what if you unintentionally mess your existing data when running databases tests on the same local server
• having to install all servers on their own is more complicated than installing just docker (configured images are provided)
• having to install all servers on their own is more resource consuming than installing just docker and running images just for the test duration
• etc.

You get a virtual environment in python for a reason, to separate it from other projects.
For servers/ machines docker is the virtualenv equivalent.

Not to mention it's still fully optional.

You want to test as you do now - sure, you got it - run tests against CI as you do now and everything works as it used to.
You don't have to install docker if you don't want to, so I don't see any risks here (adding this to a project).

[aminalaee]

Yeah I understand the use case and I am familiar with docker. Don't get me wrong.

The question is not that if we should use docker or not.
The question is why should we include infra setup in our repository? What's the benefit?

I already have a docker-compose.yml file and set everything up and run the tests. In some cases you don't even need to test with all databases, as you're changing a single backend.

This is such a general question/issue I think we can ask for feedback from @encode/maintainers here.

[collerek]

Ok, you are right - it's a valid question.

For me, the benefits are:

• providing a valid contract that contributors changes must fulfill (docker setup reflects 1 to 1 what we run in CI (for one python version of course))
• reducing the number of unnecessary commits (that could have been avoided with local tests) from contributors - If I recall correctly we don't squash
• making it easier for beginner/intermediate contributors that are not fluent in all servers/backends to setup test for all backends

If we don't want to add the code maybe a sample configuration in contributing docs?

[collerek]

@aminalaee OK, I left only contributing docs, can you review it again?

[aminalaee]

The ./scripts/lint won't run mypy, when you run ./scripts/test it will call ./scripts/check which does those checks. Needs a bit of re-wording.

[collerek]

why? It should run mypy:

databases/scripts/lint

Lines 10 to 13 in f8bff8f

	${PREFIX}autoflake --in-place --recursive databases tests
	${PREFIX}isort --project=databases databases tests
	${PREFIX}black databases tests
	${PREFIX}mypy databases

[aminalaee]

Hmmm. In all other encode projects we don't have that line.

[collerek]

Should we unify this or do I leave it as it is?

[aminalaee]

I think it's worth unifiying it, but not that important really. Up to you.

[aminalaee]

Nice 👍