Skip to content
This repository has been archived by the owner on May 22, 2018. It is now read-only.

Set the global node version in the site manifest #311

Merged
merged 1 commit into from
Jun 19, 2013
Merged

Set the global node version in the site manifest #311

merged 1 commit into from
Jun 19, 2013

Conversation

smcnabb
Copy link
Contributor

@smcnabb smcnabb commented Jun 19, 2013

It doesn't look like the global node version gets set anywhere. I had to add this change to our site manifest to fix.

Before:

Last login: Wed Jun 19 11:39:04 on ttys002
nodenv: couldn't find any version specified for use
smcnabb@mac:~ $ node --version
nodenv: couldn't find any version specified for use
smcnabb@mac:~ $ nodenv version
nodenv: couldn't find any version specified for use
smcnabb@mac:~ $ set | grep NODE
NODENV_ROOT=/opt/boxen/nodenv
NODE_PATH=/opt/boxen/nodenv/versions//lib/

After:

Last login: Wed Jun 19 11:39:14 on ttys002
smcnabb@mac:~ $ node --version
v0.10.7
smcnabb@mac:~ $ nodenv version
v0.10
smcnabb@mac:~ $ set | grep NODE
NODENV_ROOT=/opt/boxen/nodenv
NODE_PATH=/opt/boxen/nodenv/versions/v0.10/lib/

@rafaelfranca
Copy link
Member

In my opinion this should be set in your organization project.

@ocxo
Copy link
Contributor

ocxo commented Jun 19, 2013

I think having a global node version would be useful in some cases.

@smcnabb
Copy link
Contributor Author

smcnabb commented Jun 19, 2013

@rafaelfranca I would agree if there weren't node versions being installed in the site manifest file. Since there are I think it would be helpful to at least setup one as the default.

@rafaelfranca
Copy link
Member

So 👍

@rafaelfranca
Copy link
Member

I removed these version on my organization project.

rafaelfranca added a commit that referenced this pull request Jun 19, 2013
Set the global node version in the site manifest
@rafaelfranca rafaelfranca merged commit 40de1e4 into boxen:master Jun 19, 2013
@wfarr
Copy link
Contributor

wfarr commented Jun 19, 2013

I quite agree.

Defaults are nice and all, but the override for this is hack-ish at best. This change likely should be backed out and discussed further.

On Jun 19, 2013, at 11:48 AM, Rafael Mendonça França notifications@github.com wrote:

In my opinion this should be set in your organization project.


Reply to this email directly or view it on GitHub.

@rafaelfranca
Copy link
Member

I'll revert it. Thanks for your input.

rafaelfranca pushed a commit that referenced this pull request Jun 19, 2013
This reverts commit 40de1e4, reversing
changes made to 206dee9.

Reason: This make harder to override the global version if it is
required for someone.

See #311 (comment)
@smcnabb
Copy link
Contributor Author

smcnabb commented Jun 19, 2013

@wfarr Sorry, I didn't realize I was being hacky - I'm still learning boxen and puppet. I'd be happy to take another crack at this if you could suggest a better approach?

trvrb added a commit to trvrb/my-boxen that referenced this pull request Jun 20, 2013
* 'master' of https://github.com/boxen/our-boxen:
  Revert "Merge pull request boxen#311 from smcnabb/master"
  Set the global node version in the site manifest
  Add note about GitHub usernames with dashes in README
  Replace dashes by underscores in GitHub usernames
salimane added a commit to salimane/our-boxen that referenced this pull request Jun 23, 2013
* upstream/master:
  Revert "Merge pull request boxen#311 from smcnabb/master"
  Set the global node version in the site manifest
  Add note about GitHub usernames with dashes in README
  Replace dashes by underscores in GitHub usernames
@wfarr
Copy link
Contributor

wfarr commented Jul 15, 2013

@smcnabb So the part that makes this hacky is any override behavior.

What I'd like to do is have a Pull Request against the nodejs repo that adds a global_version or default_version parameter to the nodejs class, which is overridable via Hiera, that sets nodejs::global to the expected version. We can then default this to the latest stable version.

The reason this particular approach is IMO better is that for anyone forking this repo, in the future, they'll be able to simply edit a yaml file in this repo to change things like the default nodejs, ruby, etc versions. And in fact, Hiera makes it easy for users to do this for themselves with overrides. This makes customizing one's own environment super easy to understand without having to understand Puppet itself.

@henry74
Copy link

henry74 commented Jul 19, 2013

So what's the right way to do this right now?

@wfarr
Copy link
Contributor

wfarr commented Jul 19, 2013

class { 'nodejs::global':
  version => 'v0.10'
}

tarVolcano pushed a commit to tarVolcano/my-boxen that referenced this pull request Nov 13, 2013
This reverts commit 40de1e4, reversing
changes made to 206dee9.

Reason: This make harder to override the global version if it is
required for someone.

See boxen/our-boxen#311 (comment)
smh pushed a commit to smh/my-boxen that referenced this pull request Dec 7, 2013
This reverts commit 40de1e4, reversing
changes made to 206dee9.

Reason: This make harder to override the global version if it is
required for someone.

See boxen/our-boxen#311 (comment)
tsphethean pushed a commit to tsphethean/my-boxen that referenced this pull request Jun 11, 2014
This reverts commit 40de1e40f3e3c5d47d157c1f996a2a21f033a113, reversing
changes made to 206dee9965730738b0c6011e271950ebfa329d75.

Reason: This make harder to override the global version if it is
required for someone.

See boxen/our-boxen#311 (comment)
mvidaurre pushed a commit to magma-labs/boxen that referenced this pull request Apr 13, 2017
This reverts commit 40de1e40f3e3c5d47d157c1f996a2a21f033a113, reversing
changes made to 206dee9965730738b0c6011e271950ebfa329d75.

Reason: This make harder to override the global version if it is
required for someone.

See boxen/our-boxen#311 (comment)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants