-
-
Notifications
You must be signed in to change notification settings - Fork 11.4k
Nodenv variant #28955
Comments
Any chance OiNutters and wfarrs can fight it out, and one of them can rename their project? |
I don't mind what the general consensus is, obviously I'd prefer not to change mine. I think the main advantage potentially for people using @wfarr's version over mine is that the |
I spent a couple of hours last night bootstrapping a new Mac. And it didn't occur to me that there could be two libraries with the same names. And yet, there are. I second @maxnordlund suggestion that the lib that is actively maintained be changed in the brew repo. I just looked at @wfarr's contributions recently and it seems very Go-centric. I know I can just change my own formula, but this double dip is a bit confusing - maybe only to me. |
I'm fine with this change, personally. @OiNutter Does the node-build plugin support downloading precompiled binaries? If not, I think this would be a great thing to support, because there's really not any sense for most folks to compile full versions of node. If your software has this functionality, I'm more than happy to update the README on mine to consider it deprecated in favor of yours. |
@wfarr I don't think so, like I said I've pretty much just copied rbenv, changed the ruby references to node and stripped out the stuff I obviously wasn't going to need (jRuby type stuff). I can have a look at adding that in though. |
Will review a pull request to make the appropriate change once a solution is agreed upon among the affected projects. |
I think the current state of affairs is I need to test mine works with precompiled binaries. I just haven't had much free time of late to take a look at it. |
I noticed https://github.com/wfarr/nodenv hasn't been updated in a year so I figured it would be best to keep a maintained version on Homebrew. As a user, I would much prefer using @OiNutter's maintained version. That being said, I pull requested @OiNutter's version here and things seem to be looking well. Will you guys take a look? #32193 Cheers! |
It appears that both @wfarr and @OiNutter are okay with this change? (to make @OiNutter 's variant the default and give @wfarr 's variant a different name). I realize that everyone involved would like to see @OiNutter 's variant be updated to support precompiled binaries. However, I don't think precompiled-binaries shouldn't be a blocker for moving forward with the rename/change-default-formula. Granted, it's slightly annoying that each install of node now requires a full compile, but I think it's more harmful to the homebrew/nodenv community that the name confusion continue for much longer. I would like to see the rename occur ASAP and let work continue in parallel for adding precompiled-binary support to @OiNutter 's variant. There is already a nodenv/node-build#11 for getting precompiled binary support into node-build. |
|
It sounds like there's consensus from both project owners about this - so should the change be made, then? |
Yes, I'm OK with this. Can someone open a PR? Thanks! |
@jasonkarns It can be left as-is; it's fine for us to just change the version. If you can open a PR that'd be good. |
@MikeMcQuaid pr here #45578 nodenv formula also has a recommended dependency on node-build, which is added with #45576 |
This can be closed now (by #45576) |
There are two mayor variants of the nodenv script/handler, OiNutters and wfarrs. Currently the wafarr variant is in homebrew, see #28830. This is also the variant used by Boxen. However, it hasn't been updated in a year, unlike OiNutter who updated 13 days ago at the time of this writing.
I believe both should be in homebrew, but the OiNutters variant being the default since it's actively being worked upon. Perhaps wfarrs variant could be renamed to something like
nodenv-boxen
orboxen-nodev
. Another possiblility is to put it in homebrew-versions. Either way, it'll be good to have a discussion about this so future people wondering why X was chosen over Y knows the reasons.The text was updated successfully, but these errors were encountered: