-
-
Notifications
You must be signed in to change notification settings - Fork 11.3k
Conversation
See also #43714. |
+1 |
@@ -14,6 +14,7 @@ class Node < Formula | |||
end | |||
|
|||
option "with-debug", "Build with debugger hooks" | |||
option "with-icu4c", "Build with Intl (icu4c) support" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why has this been added as a separate option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It reflects this choice when configuring node:
https://github.com/nodejs/node#intl-ecma-402-support
I ported what we were doing for iojs here: https://github.com/aredridel/homebrew-iojs/blob/master/Formula/iojs.rb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It reflects this choice when configuring node:
https://github.com/nodejs/node#intl-ecma-402-support
I see. I like that there are a few different options now.
I ported what we were doing for iojs here: https://github.com/aredridel/homebrew-iojs/blob/master/Formula/iojs.rb
So if I understanding correctly, the "default" is becoming --with-intl=system-icu
, (a decision which I heartily concur with; I think its the best option), and the homebrew install flag '-with-icu4c
is to force the build to link with homebrew's 'icu4c' package? May I suggest then that the option be changed to --with-brewed-icu4c
, a la the git.rb
formula's options for openssl, ssl, and curl?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or simply remove this as an option altogether, since I don't see much reason why one should wan't to not link to the system lib, as it's perfectly adequate?
@geoff-codes I appreciate that you deleted your comment but everyone watching this thread still saw it. Please read our Code of Conduct: https://github.com/Homebrew/homebrew/blob/master/CODEOFCONDUCT.md |
@MikeMcQuaid All I can really say is that,
|
@geoff-codes That's 🆗, we appreciate the apology, thanks. |
Thanks. Having had a proper good refresher on said Code of Conduct, I quite appreciate your criticism as laid out therein:
I think if everyone held each other to account as well as you have with me just now, I'd reckon the internet would be a much more civilized place. 👍 😉 🍻 |
@asparagui I apologize for the duplicate effort: somehow I missed #43714 in the list of PRs. |
Reviewing the earlier PR by @killswitch my feeling is that the discussions and reviews are more advanced over there. |
@hmalphettes Thanks for closing this one up, and submitting the PR. Do appreciate the submission. I'm going to lock this thread as keeping tabs on two active discussion threads is enough for everyone, let alone three, but welcome politely-toned discussion in the other threads. |
Cheers!