search

iamtrask / DEPRECATED

DEPRECATED - do not use!!!
Apache License 2.0
349 stars 48 forks source link

Formalizing Conventions (Coding, commits, PRs) #5

Open samsontmr opened 6 years ago

samsontmr commented 6 years ago

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

Continuous Integration (Automated Testing, Code Quality/Style)

kevinahuber commented 6 years ago

Here's a couple things discussed in slack before:

Branching

How about this for branching-

Technical Workflow

Hierarchy

image

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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

iamtrask commented 6 years ago

@kevinahuber how we lookin?

kevinahuber commented 6 years ago

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

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.