microsoft / vsts-cordova-tasks

Streamline CI setup for your Apache Cordova, PhoneGap, Ionic, or Cordova CLI compatible app using a set of useful pre-defined build steps for VS Team Services or TFS
http://go.microsoft.com/fwlink/?LinkID=691188
Other
25 stars 27 forks source link

Remove vsts-task-lib references. #41

Closed ryuyu closed 8 years ago

ryuyu commented 8 years ago

As the vsts-task-lib will be moving to require Node 4.0, we will be unable to take a dependency on this in the tasks that need to run in node 0.12.10 to node 5.x (depending on cordova version).

This PR removes all references to the task lib from the script that will be shelled out.

Also addresses some code sharing concerns we had. Common code has been refactored and moved to lib/ and is copied into the respective locations at build time. Library file dependencies are listed in each tasks' libs.json file.

Addresses https://github.com/Microsoft/vsts-cordova-tasks/issues/16 and https://github.com/Microsoft/vsts-cordova-tasks/issues/35

@BretJohnson @qianjun22 @Chuxel

Chuxel commented 8 years ago

Okay so two thoughts:

  1. Was shelljs in our 3rd party notices file before? If not, we probably need to get it added.
  2. The only risk I see with this approach is if the VSTS team decides to move away from environment variables as the way to pass VSTS options in... or makes small changes like the env var prefixes. It may be safer to bite the bullet and pass them as command line args
ryuyu commented 8 years ago

We will probably have to update some version numbers, but shelljs 0.3.0 was included in the third party notices as shipped by vsts devops task sdk. The shelljs i tested this against was 0.6.0. We may have to update.

Chuxel commented 8 years ago

BTW - A possible easy way to send the inputs to the task that reduces the amount of work is to put all of the variables we grab in an object and then use JSON.stringify to pass the object to the downstream script (and JSON parse on the other side). You then don't need to keep track of parameter ordering or anything.

ryuyu commented 8 years ago

I updated the scripts to pass through the expected parameters in the environment.

Chuxel commented 8 years ago

LGTM!