Formalizing Conventions (Coding, commits, PRs)

[samsontmr]

Since we're growing rather rapidly, @iamtrask and I thought it'd be good to get the discussion going on formalizing some conventions.

Here are some to get the ball rolling:

Coding Conventions

Python: PEP8 Javascript: ES6(?)

Commits and PRs

Great reference used by another open source project I work on. This can be relaxed as much as we see fit.

Main points

• One logical step per commit (makes for easier reviews)
• Appropriate commit messages (same reason as above)
• Type of merge (squash or not)

Continuous Integration (Automated Testing, Code Quality/Style)

• Travis/Circle CI
• Sonar (heh)

[kevinahuber]

Here's a couple things discussed in slack before:

Branching

How about this for branching-

• master stays protected, PRs to master only.
• branch names have the following tags: bug- fixing bugs in existing code chore- updating tooling or cleanup docs- adding/updating docs feature - adding new features, work on a feature tool - adding modules/tools, development candy
• branch names follow the following structure: <tag>/<feature name>--<branch changes> i.e: feature/training--restarting or chore/webpack--update-to-2

Technical Workflow

Hierarchy

From @anoff: Names I am most familiar with are user > contributor > maintainer > core developer > (potentially TSC (tech steer committee)) maintainer + core get merge rights, whereas core devs should be taken into the loop when it comes to big technical decisions, maintainers are free to merge patch level commits

[anoff]

I'm not a fan of forced squashing. People should squash commits that are useless but I don't see a reason not to PR up to 10 commits. For javascript I'm currently leaning towards standard as the community seems to be heading that direction. Never used it before the Mine.js project and I think it is pretty intuitive. That being sad I favor es6 (syntax) + standard (codestyle).

[samsontmr]

@anoff me neither. In fact, I'd rather have more commits if it means the PR is more organized (one commit per logical change). The squashing I refer to is upon PR merge: whether to include the original commits or to compress an entire PR into a songle commit on master. Both have their merits (non-squash preserves history but requires greater discipline and enforcement in an OSS project, squash vice-versa).

Javascript: I don't work with it enough to form an opinion, so I'd love to hear yours (and everyone else's)!

@kevinahuber love branch prefixes! Also a fan of the rebase workflow.

[anoff]

What do you guys think of commitizen for commit messages? I'm always using https://www.npmjs.com/package/cz-conventional-kawaii to make sure my commit messages have a fixed format </>

However the things seem to be redundant with the branch descriptions. Advantage of using commitizen is that you can autogenerate changelogs. As you pointed out it requires discipline though :)

[samsontmr]

@anoff if this is something running locally (which it seems to be), I think we can safely let the individual devs decide if they want to use it. We could include it in a resources section or something though.

[anoff]

Well it is run locally but the idea behind is that you enforce a convention for commit messages. Whether you use the tooling to comply is up to the developer.

[kevinahuber]

I agree it is redundant to the branching model, but I wouldn't discourage it. I think it'll be super hard to enforce without limiting the barrier to entry- so my vote is that we just say be descriptive.

[samsontmr]

@anoff Yup! We'll have a convention, but whether or not the contributors chooses to use the tool to help them is up to them.

[kevinahuber]

I am making strides on a bunch of this, should have a PR tonight.

[iamtrask]

@kevinahuber how we lookin?

[kevinahuber]

@iamtrask sorry missed this! We are almost there. The points we have not addressed are:

• [ ] CI (currently experimenting to determine what will work best)
• [ ] Type of merge https://www.atlassian.com/git/articles/git-team-workflows-merge-or-rebase

I'm a fan of merging commits. I while it adds a lot of noise, it allows for better sleuthing out of the story of development.