Move all source to ES6

[steelbrain]

Instead of having two helper files, one in ES5 and one in ES6, I decided to move all to babel and use ucompiler to compile them

How to set the new env up

npm install -g ucompiler
cd ~/.atom/packages/linter-eslint
npm install

How to compile the source files

cd ~/.atom/packages/linter-eslint
ucompiler go src

EDIT

Now npm i in the project directory should install all needed deps.

Commands

Compile:

npm run compile

Publish:

npm run apm-publish -- <bump_type>

(publishing will always build a clean version of the assets)

[IanVS]

Holy linting errors. 👼

[steelbrain]

😛 Maybe because it's linting the lib folder instead of just src folder

[Arcanemagus]

If linting lib is no longer proper, you should update the build parameters, although if linting lib is causing issues you will no matter what fail Travis without a custom script.

Why shouldn't the lib folder pass a lint?

Also from the sounds of it this changes how development will work on this from now on, so I would say this should split out the Contributing section of the README.md into a CONTRIBUTING.md and update it with the new steps.

[steelbrain]

Whoever wants can take this PR from here on, I have a long todo list, including rewriting the linter UI and authoring a spec for a base debugger API

[IanVS]

I think it would be easier for us to continue working on it if it were a branch on the repo, rather than on your fork. Is there a chance you could move it to a repo branch?

Edit: maybe an easy way is to make a pull request onto a new branch of the project?

[iam4x]

Seriously ucompiler? I know its a projet from you @steelbrain but everyone use babel and everything you do can be done with babel CLI.

Any benefits for ES6 for theses files? Now we need to build the projet on releases...

[SpainTrain]

If we get a branch in this repo we can riff on, I'm happy to add the stuff I mention below:

FYI, with npm run scripts you do not need to install things globally, just add build deps like eslint and babel to devDependencies and it will work. npm automatically adds ./node_modules/.bin to the PATH in run scripts. This is better practice as well since both CI and other devs will be installing the same version of build deps.

linting

Generally one does not check built files into source control at all; those files belong in npm/apm only. Building on CI isn't too tough, I do it for all my projects. The atom testing script is the only confounding factor, but shouldn't be a problem IIRC how the script works. As mentioned, happy to mess with it if we get a riff branch going.

[Arcanemagus]

Just a note, this is a branch on this repo, he just named it steelbrain/improve.

[IanVS]

Sneaky.

[SpainTrain]

Just a note, this is a branch on this repo, he just named it steelbrain/improve.

[steelbrain]

@iam4x To answer your question, I am using babel, Ucompiler is just the base of it. Ucompiler is just a compiler which saves us from a different compile command for each file, if I recall correctly the babel cli only compiles one file at a time, but ucompiler allows you to create a .ucompilerrc and then you can just do ucompiler go src and it'll compile the files in src folder and put them in lib

[steelbrain]

About the other part of your question, If you read the PR description we had two helper files, helpers.js and es5-helpers.js :) This PR intends to solve that.

[SpainTrain]

babel cli only compiles one file at a time

nah - https://babeljs.io/docs/usage/cli/#compile-directories

[steelbrain]

@SpainTrain that's new for me 😛 , Lets use that!

[SpainTrain]

👍 I am messing with CI stuff now and can make the switch.

I do appreciate the effort to dog food your projects though =-)

[SpainTrain]

These should all be in a single override section

That example doesn't have a build step. We want the inferred installation for this "machine" type (node) because we need to run babel, so we do not wish to override. I also make use of pre and post to communicate my intention to other devs "I explicitly want to do this before npm i and this after npm i.

[SpainTrain]

PSA: I am about to squash and force push to this working branch.

[SpainTrain]

Ok, so CI and build stuff is taken care of. The specs just need to be fixed now.

[iam4x]

Exactly what @SpainTrain said 👍

Should we wait on this: atom/apm#376 to be merged into apm before switching to ES6? With this we can remove the lib folder and compiled files on the repo.

I think having compiled files on the repo is a big mistake.

[SpainTrain]

whoa, I removed the lib dir, but I guess I lost the removal in squashing. Doh! Fixed!

[SpainTrain]

Should we wait on this

That would certainly be safer, but not strictly required to remove the compiled assets from source control.

Another option would be to have CI perform publishing. The way this would work is that you add the following directive to a commit message (or PR merge message):

release v+<bump_type>

and CI would build, bump, and publish after tests ran successfully. I use this approach for my work projects because humans aren't to be trusted X-D (See https://github.com/LiveSafe/gulp-html5-lint for an example)

[iam4x]

Exactly, doing the same at work 👍

[Arcanemagus]

I rather like that idea, prevents somebody from accidentally releasing a version that isn't passing tests.

[SpainTrain]

Ugh, FYI I will need to change my approach. I was under the impression that apm used a repo of tarballs like npm, but it is just a repo of git pointers like bower (https://github.com/atom/apm#relation-to-npm). Barf. I will need to figure out a way to get the built assets into a release tag. I may have it:

1. build
2. commit assets
3. publish
4. delete assets
5. commit
6. push to master

It pollutes the git log, but keeps assets out of the repo in stable state. Any thoughts?

[Arcanemagus]

Are you sure it isn't just using the auto-generated downloads from GitHub releases?

[SpainTrain]

Are you sure it isn't just using the auto-generated downloads from GitHub releases?

Right, but the problem is the same, I think, since those tarballs are created from the tagged commit

[SpainTrain]

I suppose we could overwrite the files immediately after the publish occurs, but that seems dodgy.

[Arcanemagus]

Oh this is ugly, but seems to be the way bower people do this: https://github.com/walmartlabs/grunt-release-component

So far it's looking like waiting on atom/apm#376 is the best option if we don't want the compiled files in the repository (which we don't!).

I'm really hoping I'm misunderstanding something here...

[SpainTrain]

It is unclear what atom/apm#376 buys us other than some sugar? With that feature we would build in a pre-publish hook and delete assets in a post-publish hook. Without that feature, we just do those steps in the CI config. Am I missing a feature of that PR?

[Arcanemagus]

Hmmm, I assumed that let us get around the issue somehow... just looked into it and it really doesn't get us anything.

[steelbrain]

so I did some investigation about the overhead of IPC calls, turns out it's almost none after the first call, see https://github.com/steelbrain/benchmark-ipc#inter-process

[steelbrain]

Also, this PR is too large, I've decided to create a branch on this repo and do stuff one-by-one to make the review easier