Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support old node versions #30

Merged
merged 12 commits into from
Sep 16, 2016
Merged

Support old node versions #30

merged 12 commits into from
Sep 16, 2016

Conversation

kevinsawicki
Copy link
Contributor

@kevinsawicki kevinsawicki commented Sep 12, 2016

This pull requests adds babel support for support old node versions (0.10/0.12).

This precompiles the source files using babel so it does have the downside of not using the native ES6 support that is built into newer versions of node but it does still allow the source files to use these constructs making for cleaner code.

And since the new constructs used, arrow functions and classes, compile down pretty nicely, the resulting babel generated code is still pretty readable.

Refs electron-userland/electron-prebuilt#190
Closes #27
Closes #28

/cc @malept 👀

@malept
Copy link
Member

malept commented Sep 12, 2016

@kevinsawicki I think you're going to have to do something similar to sumchecker, if you don't mind.

@kevinsawicki
Copy link
Contributor Author

I think you're going to have to do something similar to sumchecker, if you don't mind.

Yeah, will do 👍

@malept
Copy link
Member

malept commented Sep 12, 2016

@kevinsawicki
Copy link
Contributor Author

Well, that's different: https://travis-ci.org/electron-userland/electron-download/jobs/159396306

Yeah, these were actually occurring on the builds of #29 as well, https://travis-ci.org/electron-userland/electron-download/jobs/159380783

Super weird, I was chalking it up to a Travis bug/machine issue since I couldn't reproduce locally and the introduction of tape causing these crashes seems unlikely to be related (famous last words).

@kevinsawicki kevinsawicki force-pushed the support-old-node-versions branch from 50cd3c4 to 6147a02 Compare September 13, 2016 00:09
@kevinsawicki
Copy link
Contributor Author

The crashes on the mac workers were addressed by #32

@malept
Copy link
Member

malept commented Sep 13, 2016

The crashes on the mac workers were addressed by #32

That is bizarre.

@malept
Copy link
Member

malept commented Sep 13, 2016

AppVeyor also needs to build against Node 0.10 and 0.12.

@kevinsawicki kevinsawicki force-pushed the support-old-node-versions branch from 6d9ba45 to 539789a Compare September 16, 2016 20:28
@kevinsawicki kevinsawicki force-pushed the support-old-node-versions branch from 3a8ddea to d91d74e Compare September 16, 2016 20:39
@@ -25,12 +28,15 @@
"home-path": "^1.0.1",
"minimist": "^1.2.0",
"nugget": "^2.0.0",
"path-exists": "^3.0.0",
"path-exists": "^2.1.0",
"rc": "^1.1.2",
"semver": "^5.3.0",
"sumchecker": "^1.1.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO sumchecker should be bumped to enforce a minimum version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So sorry, I accidentally deleted your commit while rebasing and force-pushing, my apologies, will bring it back shortly 😥

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So weird to get a notification for my own commit...

@kevinsawicki kevinsawicki merged commit cb9c1fe into master Sep 16, 2016
@kevinsawicki kevinsawicki deleted the support-old-node-versions branch September 16, 2016 21:18
@kevinsawicki
Copy link
Contributor Author

Thanks @malept for your help, review, and sumchecker updates on this one 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants