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

node 6.0.0 #633

Closed
wants to merge 4 commits into from
Closed

node 6.0.0 #633

wants to merge 4 commits into from

Conversation

xeoneux
Copy link
Contributor

@xeoneux xeoneux commented Apr 26, 2016

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same formula update/change?
  • Have you built your formula locally prior to submission with brew install <formula> (where <formula> is the name of the formula you're submitting)?
  • Does your submission pass brew audit --strict --online <formula> (after doing brew install <formula>)?

Description

Update node to v6.0.0

sha256 "472ae46a205fb65cd784b22825b7354cddb4da5f46c1974272be43e55745ce6e"
version "6.0.0-rc.3"
end
# devel do
Copy link
Contributor

Choose a reason for hiding this comment

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

Just remove this block entirely if there's no new development release.

@@ -47,7 +41,7 @@ class Node < Formula
end

resource "icu4c" do
url "https://ssl.icu-project.org/files/icu4c/56.1/icu4c-56_1-src.tgz"
url "https://ssl.icu-project.org/files/icu4c/57.1/icu4c-57_1-src.tgz"
mirror "https://ftp.mirrorservice.org/sites/download.qt-project.org/development_releases/prebuilt/icu/src/icu4c-56_1-src.tgz"
Copy link
Contributor

Choose a reason for hiding this comment

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

The mirror needs to be updated too.

@xeoneux
Copy link
Contributor Author

xeoneux commented Apr 26, 2016

@dunn MACOSX_DEPLOYMENT_TARGET changed to 10.7. Does it affect the version requirement?

@xeoneux
Copy link
Contributor Author

xeoneux commented Apr 26, 2016

All tests pass except azure-cli due to changes in Node.js v6 API. Issue over at Azure/azure-xplat-cli.

@dunn
Copy link
Contributor

dunn commented Apr 26, 2016

Thanks for opening the Azure PR! Hopefully they'll merge it quickly.

@chrmoritz
Copy link
Contributor

chrmoritz commented Apr 27, 2016

The ICU 57 upgrade didn't made it into node v6.0.0, so it should be reverted from this PR (as it's not supported).

You also have to add a revision to the azure-cli formula too, so that the native addon part of it can be recompiled against v8 5.0 shiped with node6.
Edit: And of cause we need either a floating patch with the node 6 fix too (or wait for a new release if a quick release with a fix for it is planned).

The MACOSX_DEPLOYMENT_TARGET change was made to allow a libuv upgrade to v2.0 to be semver minor. Until this upgrade lands later in a semver minor v6.x release node 6 will still be compatible with older OS X versions.

@DomT4 DomT4 added the needs response Needs a response from the issue/PR author label Apr 27, 2016
@ghost ghost removed the needs response Needs a response from the issue/PR author label Apr 27, 2016
@jbergstroem
Copy link
Contributor

How about introducing support for building against a shared c-ares?

@chrmoritz
Copy link
Contributor

chrmoritz commented Apr 27, 2016

@jbergstroem: Is this comment nodejs/node#5775 (comment) still valid? If not adding an option to build against a shared c-ares shouldn't be a problem.

@jbergstroem
Copy link
Contributor

@chrmoritz yes, I've landed shared support against c-ares. Additionally, if you want, building against a shared zlib and http-parser should work fine as well.

@xeoneux
Copy link
Contributor Author

xeoneux commented Apr 27, 2016

Azure/azure-xplat-cli#2821 got merged into base branch. Are we waiting for a release?

@xeoneux
Copy link
Contributor Author

xeoneux commented Apr 27, 2016

Also, check #638 for my motivation to keep npm packages out of homebrew.

@DomT4
Copy link
Member

DomT4 commented Apr 27, 2016

Azure/azure-xplat-cli#2821 got merged into base branch. Are we waiting for a release?

No need to wait. Can backport the patch with a link to the upstream commit and explanation it can be removed on next stable release, or simply use the commit diff as a patch do if it applies cleanly with the older release.

@LinusU
Copy link
Contributor

LinusU commented Apr 27, 2016

@DomT4 Can we merge this now then?

@DomT4 DomT4 closed this in b8b57c7 Apr 27, 2016
@DomT4
Copy link
Member

DomT4 commented Apr 27, 2016

Seems reasonable. I'll deal with the azure-cli issue shortly after merging.

Merged in b8b57c7. Thank you for your contribution to Homebrew @xeoneux; we appreciate it! 😺

@LinusU
Copy link
Contributor

LinusU commented Apr 27, 2016

Awesome 🎉

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants