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

chore(*): use Yarn for dependency management #15435

Merged
merged 15 commits into from
Nov 29, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .nvmrc
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
4

6
17 changes: 5 additions & 12 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
language: node_js
sudo: false
node_js:
- '4.4'
- '6'

cache:
directories:
Expand Down Expand Up @@ -36,19 +36,12 @@ addons:
packages:
- g++-4.8

install:
# Check the size of caches
- du -sh ./node_modules ./bower_components/ ./docs/bower_components/ || true
# - npm config set registry http://23.251.144.68
# Disable the spinner, it looks bad on Travis
- npm config set spin false
# Log HTTP requests
- npm config set loglevel http
#- npm install -g [email protected]
# Install npm dependencies and ensure that npm cache is not stale
- npm install
before_install:
- curl -o- -L https://raw.githubusercontent.com/yarnpkg/yarn/2a0afc73210c7a82082585283e518eeb88ca19ae/scripts/install-latest.sh | bash -s -- --version 0.17.9
- export PATH=$HOME/.yarn/bin:$PATH

before_script:
- du -sh ./node_modules ./bower_components/ ./docs/bower_components/ || true
- ./scripts/travis/before_build.sh

script:
Expand Down
10 changes: 7 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,9 @@ We have very precise rules over how our git commit messages can be formatted. T
readable messages** that are easy to follow when looking through the **project history**. But also,
we use the git commit messages to **generate the AngularJS change log**.

The commit message formatting can be added using a typical git workflow or through the use of a CLI wizard ([Commitizen](https://github.com/commitizen/cz-cli)). To use the wizard, run `npm run commit` in your terminal after staging your changes in git.
The commit message formatting can be added using a typical git workflow or through the use of a CLI
wizard ([Commitizen](https://github.com/commitizen/cz-cli)). To use the wizard, run `yarn run commit`
in your terminal after staging your changes in git.

### Commit Message Format
Each commit message consists of a **header**, a **body** and a **footer**. The header has a special
Expand All @@ -229,7 +231,8 @@ Any line of the commit message cannot be longer 100 characters! This allows the
to read on GitHub as well as in various git tools.

### Revert
If the commit reverts a previous commit, it should begin with `revert: `, followed by the header of the reverted commit. In the body it should say: `This reverts commit <hash>.`, where the hash is the SHA of the commit being reverted.
If the commit reverts a previous commit, it should begin with `revert: `, followed by the header of the reverted commit.
In the body it should say: `This reverts commit <hash>.`, where the hash is the SHA of the commit being reverted.

### Type
Must be one of the following:
Expand Down Expand Up @@ -266,7 +269,8 @@ The body should include the motivation for the change and contrast this with pre
The footer should contain any information about **Breaking Changes** and is also the place to
[reference GitHub issues that this commit closes][closing-issues].

**Breaking Changes** should start with the word `BREAKING CHANGE:` with a space or two newlines. The rest of the commit message is then used for this.
**Breaking Changes** should start with the word `BREAKING CHANGE:` with a space or two newlines.
The rest of the commit message is then used for this.

A detailed explanation can be found in this [document][commit-message-format].

Expand Down
72 changes: 56 additions & 16 deletions Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,63 @@ var path = require('path');
var e2e = require('./test/e2e/tools');

var semver = require('semver');
var fs = require('fs');
var exec = require('shelljs').exec;
var pkg = require(__dirname + '/package.json');

var useNodeVersion = fs.readFileSync('.nvmrc', 'utf8');
if (!semver.satisfies(process.version, useNodeVersion)) {
throw new Error('Invalid node version; please use node v' + useNodeVersion);
// Node.js version checks
if (!semver.satisfies(process.version, pkg.engines.node)) {
reportOrFail('Invalid node version (' + process.version + '). ' +
'Please use a version that satisfies ' + pkg.engines.node);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Missing trailing period 😁
Nit: I would make the message a little "softer". For example, the build will (hopefully) work fine with v7+; calling it an "invalid version" sounds "strong".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're too kind! If you feel strongly about it I will let you fix up the messaging in a future commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Along with the missing full stops

}

// Yarn version checks
var expectedYarnVersion = pkg.engines.yarn;
var currentYarnVersion = exec('yarn --version', {silent: true}).stdout.trim();
if (!semver.satisfies(currentYarnVersion, expectedYarnVersion)) {
reportOrFail('Invalid yarn version (' + currentYarnVersion + '). ' +
'Please use a version that satisfies ' + expectedYarnVersion);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Missing trailing period.

}

// Grunt CLI version checks
var expectedGruntVersion = pkg.engines.grunt;
var currentGruntVersions = exec('grunt --version', {silent: true}).stdout;
var match = /^grunt-cli v(.+)$/m.exec(currentGruntVersions);
if (!match) {
reportOrFail('Unable to compute the current grunt-cli version. We found:\n' +
currentGruntVersions);
} else {
if (!semver.satisfies(match[1], expectedGruntVersion)) {
reportOrFail('Invalid grunt-cli version (' + match[1] + '). ' +
'Please use a version that satisfies ' + expectedGruntVersion);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Missing trailing period.

}
}

// Ensure Node.js dependencies have been installed
if (!process.env.TRAVIS && !process.env.JENKINS_HOME) {
var yarnOutput = exec('yarn install');
if (yarnOutput.code !== 0) {
throw new Error('Yarn install failed: ' + yarnOutput.stderr);
}
}


module.exports = function(grunt) {
//grunt plugins

// this loads all the node_modules that start with `grunt-` as plugins
require('load-grunt-tasks')(grunt);

// load additional grunt tasks
grunt.loadTasks('lib/grunt');
grunt.loadNpmTasks('angular-benchpress');

// compute version related info for this build
var NG_VERSION = versionInfo.currentVersion;
NG_VERSION.cdn = versionInfo.cdnVersion;
var dist = 'angular-' + NG_VERSION.full;

if (versionInfo.cdnVersion == null) {
throw new Error('Unable to read CDN version, are you offline or has the CDN not been properly pushed?');
throw new Error('Unable to read CDN version, are you offline or has the CDN not been properly pushed?\n' +
'Perhaps you want to set the NG1_BUILD_NO_REMOTE_VERSION_REQUESTS environment variable?');
}

//config
Expand Down Expand Up @@ -292,10 +329,9 @@ module.exports = function(grunt) {
},

shell: {
'npm-install': {
command: 'node scripts/npm/check-node-modules.js'
'install-node-dependencies': {
command: 'yarn'
Copy link
Member

Choose a reason for hiding this comment

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

Won't this take more time that simply comparing two files (s was happening before). I am not very keen on sacrificing speed for every task for this 😕

Copy link
Member

@mgol mgol Nov 28, 2016

Choose a reason for hiding this comment

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

It shouldn't be a problem. The following was run on the angular.js repo with npm & yarn caches cleared:

$ time yarn --ignore-scripts
yarn --ignore-scripts  28.22s user 21.07s system 125% cpu 39.178 total
$ time yarn --ignore-scripts
yarn --ignore-scripts  6.49s user 2.68s system 120% cpu 7.583 total
$ time yarn --ignore-scripts
yarn --ignore-scripts  0.87s user 0.07s system 70% cpu 1.321 total
$ time yarn --ignore-scripts
yarn --ignore-scripts  0.86s user 0.07s system 70% cpu 1.313 total

Copy link
Member

Choose a reason for hiding this comment

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

Right 👍

When trying this myself, I stopped after the 2nd time 😃

Copy link
Member

Choose a reason for hiding this comment

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

Could you try? It'd be good to make sure it's that quick not only on *NIX-es.

Copy link
Member

Choose a reason for hiding this comment

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

I did try of course. I didn't take your word for it 😛
It "stabilizes" at 2secs on my machine, which is OK (for me at least).

Copy link
Member

Choose a reason for hiding this comment

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

I wonder why it's not that fast on the second try, though. I'd expect it to behave identically in all situations where cache is populated and dependencies are correct so something looks wrong here.

Copy link
Member

Choose a reason for hiding this comment

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

Let's see: yarnpkg/yarn#2054

},

'promises-aplus-tests': {
options: {
stdout: false,
Expand All @@ -322,13 +358,6 @@ module.exports = function(grunt) {
}
});

// global beforeEach task
if (!process.env.TRAVIS) {
grunt.task.run('shell:npm-install');
}



//alias tasks
grunt.registerTask('test', 'Run unit, docs and e2e tests with Karma', ['eslint', 'package', 'test:unit', 'test:promises-aplus', 'tests:docs', 'test:protractor']);
grunt.registerTask('test:jqlite', 'Run the unit tests with Karma' , ['tests:jqlite']);
Expand All @@ -350,3 +379,14 @@ module.exports = function(grunt) {
grunt.registerTask('ci-checks', ['ddescribe-iit', 'merge-conflict', 'eslint']);
grunt.registerTask('default', ['package']);
};


function reportOrFail(message) {
if (process.env.TRAVIS || process.env.JENKINS_HOME) {
throw new Error(message);
} else {
console.log('===============================================================================');
console.log(message);
console.log('===============================================================================');
}
}
2 changes: 1 addition & 1 deletion TRIAGING.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ You can mention him in the relevant thread like this: `@btford`.

> Thanks for submitting this issue!
> Unfortunately, we don't think this functionality belongs in core.
> The good news is that you could easily implement this as a third-party module and publish it on Bower and/or npm.
> The good news is that you could easily implement this as a third-party module and publish it on Bower and/or to the npm repository.


## Assigning Work
Expand Down
208 changes: 0 additions & 208 deletions changelog.js

This file was deleted.

Loading