Process discussion

[lukesneeringer]

@stutrek @davecoates @udnisap

I have merged #24 and we now have automated testing on all pull requests. I also set up master as a protected branch in GitHub, which means it is no longer possible to push to it directly; anything has to be a pull request and Travis tests must pass.

I have not set up Travis to auto-publish to npm. I can do that if we want, although I might want some coaching from @udnisap about the right way to do that.

I also think it would be useful to have a couple of process discussions. What should our rule on PR merging be? This is a pretty small project, so I think that, in general, the rule should be a +1 from any other person.

I would also like to take a pass through the code and add more commenting (and probably a linter), as well as do a thorough sweep through the documentation. Would that be something that you all would welcome, or find annoying?

Finally, would it be valuable to have a (presumably low volume) Slack (or similar) channel, or do we want to continue to use GitHub for discussion? I know I am not always the best at checking GitHub; not sure if that is a concern for others.

[stutrek]

Your work is great, thanks for setting that back up! I thought it was going to transfer with the repo.

For autopublishing, in the past I've set it up so that PRs need to be labelled major/minor/patch and it versions and publishes. Once we have all these PRs merged it'll be great, but I'm glad we didn't release like 10 minor versions with the transfer. I'm a collaborator on our npm project, LMK if you want/need to be added.

There's a process repo in this org that's based on thoughtbot. We can change it as needed. It's basically a +1 from someone plus a week to give people time to respond.

I am all for some linting, as long as the rules are very pragmatic. Instead of making reviews conform to a style, I'd rather add an autoformatter, it's reasonable to ask people to run esformatter on the repo when they make a PR.

100% for a sweep on the docs and some more comments. It might be out of the scope of this, but it would also be cool to have some basic lessons in types on the wiki, since this repo could introduce many people to types.

I just made a slack channel, if it's anything but low volume we'll have to figure something else out. https://typed-immutable.slack.com

[lukesneeringer]

For autopublishing, in the past I've set it up so that PRs need to be labelled major/minor/patch and it versions and publishes.

Can you explain to me how that works? It is derived from the PR label? (Does Travis have access to that?)

I'm a collaborator on our npm project, LMK if you want/need to be added.

I would like to be (I am lukesneeringer there too), if for no other reason than I think having a couple people who are able to do this stuff would help.

[lukesneeringer]

I just made a slack channel, if it's anything but low volume we'll have to figure something else out. https://typed-immutable.slack.com

I cannot make my own account; you will have to invite me (and the others).

[stutrek]

For PR versioning I have a Jenkins job that looks for the close event on PRs and reads the labels looking for major/minor/patch, then versions it and pushes to master. I'm sure travis has access too, but I've never even tried to set it up. What I have wouldn't work with a locked master branch.

We could ask contributors to run npm version or make a separate PR that does it. My guess is that udnisap's job looks for changes to package.json and versions when it changes.

I added you as an NPM collaborator.

The "simple" solution for making a slack team that people can sign up for involves heroku 😕. Otherwise people will have to request from here then join the discussion on slack. I'm actually ok with using github, since once we're done with the transition this project will probably move at a glacial pace.

[lukesneeringer]

For PR versioning I have a Jenkins job that looks for the close event on PRs and reads the labels looking for major/minor/patch, then versions it and pushes to master. I'm sure travis has access too, but I've never even tried to set it up. What I have wouldn't work with a locked master branch.

I will see what I can come up with.

Incidentally, it is possible to exempt things from the lock, so if we have a tool that does what you describe, we could give it a bypass.

I'm actually ok with using github, since once we're done with the transition this project will probably move at a glacial pace.

That is fine with me.

[udnisap]

I have a different approach to versioning. I follow semantic versioning which will have these in each commits and will have the change logs tags and everything generated from the commits. Every commit needs to have a format which we can are enforcing through git hook.

https://github.com/commitizen/cz-cli is used to identify the type of the commit and http://semver.org/ on the version numbers. Basically version is x.y.z z incremented when there are fixes, y is incremented when there are features and x is incremented when there are breaking changes.

My setup is use commitezen (used in angular) and get the changes from the commit it self and then do everything else from travis. I have the exact setup for my projects. If you send a PR it will check for the messages and then based on that it will trigger a release when it is merged.

For example
https://github.com/udnisap/debuk
https://github.com/udnisap/debuk/releases
npmjs.com/package/debuk

for linting I use airbnb and you want be able to commit to repo with issues or tests failing with git hooks. I can set these up to this repo as well. But I think I need to be an owner to configure some of these and a collaborator on the npm project to push to.

[stutrek]

You're an owner now, @udnisap.

If someone sends us a PR could they do whatever they normally do then make a single commit at the end with the right format for versioning?

Can travis do auto formatting? I don't want to turn contributors away because they like to write code in a different style than we do, better to fix it automatically.

[udnisap]

If someone sends us a PR could they do whatever they normally do then make a single commit at the end with the right format for versioning?

Yes. Well every commit needs to be in that format. Not the last. With a ghook the can only commit with that style. So we are good.

Can travis do auto formatting? I don't want to turn contributors away because they like to write code in a different style than we do, better to fix it automatically.

Not all but only the trivial once. check eslint --fix I think we should enforce a common guideline so that the code can be understood by anyone.

[stutrek]

I am firmly against requiring people to use a specific style for commit messages. I know that if I did a lot of work with the wrong commit format, then made a PR and they asked me to change the messages I would politely let them know that they're welcome to take or leave the work and let the PR rot. I also know that I wouldn't install anyone's githooks, I'd just start doing what I needed to do without thinking about it. There are other solutions and we should find another.

I have never seen a discussion that involves linting or style issues be productive (aside from teaching what's wrong with globals, etc. which is solved by ES6). There are things that are important for readability, like not using nested ternaries, or picking good variable names, but trailing whitespace should never stop someone from helping us out. That's why I think we should make npm run format that runs esformatter. Then there are no disagreements about formatting and it's trivial for a new contributor to run as the last step of a PR without reading an extensive style guide.

[lukesneeringer]

I do not love commit message style requirements either, although I may not be as against them as @stutrek is (-0, not -1).

I also generally agree that we should be loose on style. The best way to handle style issues is "be consistent with the file you are in". This is not a big enough project to require a strict style-guide, and the JavaScript community has tons of different preferences (unlike, say, in Python, where it is pretty easy to just say "PEP 8" and be done).

[udnisap]

Well then we can relax them but someone needs to do the change log and releases manually.
With the setup i was referring you will have everything setup with a npm install no other configurations at all and without the conventions satisfied you wont be able to commit. so once you are adhere to it. everything else works out for you.

[stutrek]

If it can ignore commits that don't match the format I'm in favor. We could write a commit message for a new contributor and ask them to add it to the end.