Skip to content
This repository has been archived by the owner on Sep 4, 2024. It is now read-only.

Select node version #306

Merged
merged 5 commits into from
Jan 17, 2013
Merged

Select node version #306

merged 5 commits into from
Jan 17, 2013

Conversation

amitapl
Copy link
Contributor

@amitapl amitapl commented Jan 15, 2013

Fixes for issues: #305 and #254.
Moved node version selection from the node deployment logic in kudu service to the deployment script generator (when generating a node deployment script).
This way:

  1. Custom deployment script has all the logic required for deploying (including node version selection).
  2. npm is run using the selected node version.

Number 2 is only for the new generator deployment logic and not for the original one, since we will switch this sprint (or the next) and until then users can use the custom script if they require this support.

@@ -25,6 +24,9 @@ public class CustomBuilder : ISiteBuilder
private const string PreviousManifestPath = "PREVIOUS_MANIFEST_PATH";
private const string NextManifestPath = "NEXT_MANIFEST_PATH";
private const string MSBuildPath = "MSBUILD_PATH";
private const string KuduSyncCommandKey = "KUDU_SYNC_COMMAND";
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to avoid duplicating all these tokens for each mode. Maybe make them internal in one of them and get them there from the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although there are still some duplication, in the long run the intention is to remove the old deployment logic and with it all this duplication.

Amit Apple added 5 commits January 16, 2013 16:41
…deployment and update selectNodeVersion.js script.
…e it's not going to use the version selection for npm.

Instead updated selectNodeVersion.js script to have backwards compatibility (temp dir is optional).
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.

2 participants