Add contributing policy

[bennuttall]

We should add a CONTRIBUTING.md file in the root containing the rules for contributors to follow. GitHub's convention means they're prompted to read this before opening an issue or creating a PR.

It should contain any actual rules like PEP8 compliance, style guide, stuff about compatibility, etc. and general house rules.

So - what do we think they should be?

[jvlomax]

pep8 (with cammelCase) is all we need:p

In seriousnes:

• PEP8
• Function names should mirror ingame commands where possible.
• If a position is ever passed around, pass it as a vector, not a tuple,
• if you rename a basic function, add it to __init__ so the old function can still be reached, to keep compatability with old scripts
• KISS

[ghickman]

@bennuttall – Great idea.

I agree with all @jvlomax's points but would also add:

• Sort imports with isort. It can be run under tox (aside: we should expand the test suite to use that with travis) and is good at avoiding import ordering debates since it's a Standard To Live By.

[bennuttall]

We're leaving camelCase in for the legacy functions only, right?

Are we adding snake_case variants of legacy functions in too?

[doismellburning]

if you rename a basic function, add it to __init__ so the old function can still be reached, to keep compatability with old scripts

I would rather we stated desired results rather than implementation method; something like "Ensure you do not break backwards-compatibility if renaming/deleting anything"

[ghickman]

@bennuttall, @doismellburning – Agree with you both, lets codify this as:

• backwards compatibility must be maintained unless you have a very compelling reason
• snake case names as per PEP8

[dbrgn]

I disagree with the vectors. They're harder to understand than passing in x, y and z coordinates. Additionally you then need to differentiate between 2d and 3d coordinates.

As for PEP8, could we set 79 columns as soft limit and 99 as hard limit? I found that 79 sometimes makes the code harder to read than if you go over that limit by a little bit.

Also, people should add themselves to AUTHORS.md whenever they create a pull request.

(Edit: More arguments concerning the vector thing in #10).

[doismellburning]

I can't help but recall @Lukasa mentioning how requests deliberately doesn't follow PEP8, and iirc that specifically included line length limit (+ continuation indentation), and I'm inclined to agree

[dbrgn]

Django has a similar guideline: https://docs.djangoproject.com/en/1.5/internals/contributing/writing-code/coding-style/

Unless otherwise specified, follow PEP 8.

You could use a tool like pep8 to check for some problems in this area, but remember that PEP 8 is only a guide, so respect the style of the surrounding code as a primary goal.

One big exception to PEP 8 is our preference of longer line lengths. We’re well into the 21st Century, and we have high-resolution computer screens that can fit way more than 79 characters on a screen. Don’t limit lines of code to 79 characters if it means the code looks significantly uglier or is harder to read.

In other projects, I always use this guideline:

PEP8 via flake8 with max-line-width set to 99 and E126-E128 ignored.

[ghickman]

@dbrgn @jvlomax – Can I confirm you're both talking about a Vector as in the Vec3 class from vec3.py?

@dbrgn @doismellburning – PEP8 was modified recently to add a caveat on line length (along with some other changes) stating projects can go up to 99 characters. I'd be pro on taking this route too as I agree a hard limit of 80 characters can make the code less readable in places.

@dbrgn – Could you add what E126-E128 are please so it's explicit here?

[dbrgn]

@dbrgn @jvlomax – Can I confirm you're both talking about a Vector as in the Vec3 class from vec3.py?

Yes. Or actually, a general concept of a position type.

@dbrgn – Could you add what E126-E128 are please so it's explicit here?

They all concern continuation indentation, as @doismellburning already mentioned:

• E126: continuation line over-indented for hanging indent
• E127: continuation line over-indented for visual indent
• E128: continuation line under-indented for visual indent

[ghickman]

@dbrgn @jvlomax – I think separate parameters is likely to be easier to work with for new users who are unlikely to know or understand the concept of a vector. I had to go look it up and I've been programming for well over a decade!

@dbrgn – Yup, totally agree on ignoring those with flake8 then. Wrapping all of these things up in tox & travis should make contributing even easier and codified.

[dbrgn]

@dbrgn – Yup, totally agree on ignoring those with flake8 then. Wrapping all of these things up in tox & travis should make contributing even easier and codified.

OK, I'll do that tonight in #13 and post a pull request for review :)

[doismellburning]

I am massively pro:

• Codifying whatever subset of pep8 we want
• Failing the build (eventually) if pep8 validation fails

[dbrgn]

Also, as a community based project, can we agree that no contribution should land in the main code base without a pull request and at least 1 review with a good-to-go?

[doismellburning]

Absolutely

[ghickman]

Most definitely. (and I apologise for no doubt breaking this rule before now!)