aio: remove gulp tasks #14889

petebacondarwin · 2017-03-02T21:19:27Z

This is built upon #14868. Only the last commit is specific to this PR.

IgorMinar

the CI is failing

IgorMinar · 2017-03-03T07:39:40Z

aio/src/app/shared/link.directive.spec.ts

@@ -1,4 +1,7 @@
 import { LinkDirective } from './link.directive';

-describe('LinkDirective', () => {
+xdescribe('LinkDirective', () => {


don't use xdescribe simply remove the second argument from all nested its and you'll get the same effect but the its will be marked as pending rather than disabled.

There is a bug in zone.js. See my other comment

IgorMinar · 2017-03-03T07:40:06Z

aio/src/app/navigation/navigation.service.spec.ts

+  });
+
+  xit('should fetch the navigation views', () => {});
+  xit('should compute the navigation map', () => {});


just remove the second arg

@IgorMinar It doesn't work because there is a bug in zone.js:

/** * Gets a function wrapping the body of a Jasmine `it/beforeEach/afterEach` block to * execute in a ProxyZone zone. * This will run in `testProxyZone`. The `testProxyZone` will be reset by the `ZoneQueueRunner` */ function wrapTestInZone(testBody: Function): Function { // The `done` callback is only passed through if the function expects at least one argument. // Note we have to make a function with correct number of arguments, otherwise jasmine will // think that all functions are sync or async. return (testBody.length == 0) ? function() { return testProxyZone.run(testBody, this); } : function(done) { return testProxyZone.run(testBody, this, [done]); }; }

Note that it doesn't check to see if the function body is defined before reading its length.

I have a PR for it: angular/zone.js#659
But the zone.js project has problems of its own :-(

IgorMinar · 2017-03-03T07:44:31Z

aio/package.json

-    "lint": "yarn run check-env && ng lint",
+    "ng": "yarn check-env && ng",
+    "start": "yarn check-env && ng serve",
+    "build": "yarn check-env && yarn docs && ng build -prod -aot -sm",


-aot the default

Does it work with single dashes? I thought you needed double dash for non-shortcut options.

Should we have separate yarn scripts for dev build vs prod build? Or do we always want the prod build?

Why would we "build" a dev version?
If I am doing development work then I use "ng serve"

Point taken 😃

IgorMinar · 2017-03-03T07:44:56Z

aio/package.json

-    "pre~~deploy": "yarn run check-env && ng build --prod",
+    "e2e": "yarn check-env && ng e2e --no-webdriver-update",
+    "deploy-staging": "firebase use staging --token \"$FIREBASE_TOKEN\" && yarn ~~deploy",
+    "pre~~deploy": "yarn check-env && ng build --prod",


is it --prod or -prod?

From the Angular CLI help usage:

--target (String) (Default: development) Defines the build target. aliases: -t <value>, -dev (--target=development), -prod (--target=production), --target <value>, --target <value>

So it appears to be -prod. I'll fix this

The search results indicate otherwise: --prod vs -prod
Could it be a typo in the help message?

gkalpak · 2017-03-03T08:44:47Z

aio/check-env.js

+  requiredNodeVersion: engines.node,
+  requiredNpmVersion: engines.npm,
+  requiredYarnVersion: engines.yarn
+});


Why do we need a separate check-env.js?
Is it so that we don't rely on stuff outside the aio/ directory?

BTW, I would prefer if such scripts were not on the root-level. We already have a scripts/ directory 😉

we don't have a scripts folder in aio/.
I am cool with moving this to the top level folder.

Right. We don't have right now. But we will have one when #14330 lands 😉

gkalpak · 2017-03-03T08:46:41Z

aio/package.json

-    "lint": "yarn run check-env && ng lint",
+    "ng": "yarn check-env && ng",
+    "start": "yarn check-env && ng serve",
+    "build": "yarn check-env && yarn docs && ng build -prod -aot -sm",


Does it work with single dashes? I thought you needed double dash for non-shortcut options.

gkalpak · 2017-03-03T08:49:20Z

aio/package.json

-    "lint": "yarn run check-env && ng lint",
+    "ng": "yarn check-env && ng",
+    "start": "yarn check-env && ng serve",
+    "build": "yarn check-env && yarn docs && ng build -prod -aot -sm",


Should we have separate yarn scripts for dev build vs prod build? Or do we always want the prod build?

gkalpak · 2017-03-03T08:51:17Z

aio/package.json

-    "deploy-staging": "firebase use staging --token \"$FIREBASE_TOKEN\" && yarn run ~~deploy",
-    "pre~~deploy": "yarn run check-env && ng build --prod",
+    "e2e": "yarn check-env && ng e2e --no-webdriver-update",
+    "deploy-staging": "firebase use staging --token \"$FIREBASE_TOKEN\" && yarn ~~deploy",


Wow! We don't need run anymore with yarn? 🕺

Yes that is something that yarn really got right!

petebacondarwin · 2017-03-03T11:27:51Z

Addressed all comments, rebased on master, aio tasks in the Travis build now pass.

Generate the docs with `yarn docs`. Test the doc generation code with `yarn docs-test` The docs are automatically built as part of the `yarn build` task, so there is no need to rebuild them in the test_aio.sh file

IgorMinar · 2017-03-05T16:19:12Z

@petebacondarwin I rebased your PR and pushed to your branch. I'm about to merge this in if CI passes, so please do not make more changes in this branch

angular-automatic-lock-bot · 2019-09-11T00:22:37Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

petebacondarwin added the comp: aio label Mar 2, 2017

googlebot added the cla: yes label Mar 2, 2017

petebacondarwin requested a review from IgorMinar March 2, 2017 21:19

IgorMinar suggested changes Mar 3, 2017

View reviewed changes

gkalpak reviewed Mar 3, 2017

View reviewed changes

petebacondarwin force-pushed the aio-remove-gulp branch 2 times, most recently from 801ce6b to 9f68109 Compare March 3, 2017 11:19

petebacondarwin force-pushed the aio-remove-gulp branch from 9f68109 to 621e765 Compare March 4, 2017 13:00

petebacondarwin added 6 commits March 5, 2017 08:14

refactor(aio): reorganize search service for easier testing

0aa057f

test(aio): add placeholder test descriptions

397ab94

test(aio): reimplemented all the commented-out unit tests

659fb3c

build(aio): upgrade to angular 4.0.0-rc.2

02f7767

build(aio): move gulp tasks to package.json

5723ff2

Generate the docs with `yarn docs`. Test the doc generation code with `yarn docs-test` The docs are automatically built as part of the `yarn build` task, so there is no need to rebuild them in the test_aio.sh file

build: allow check-environment.js to be run directly

3a8583e

IgorMinar force-pushed the aio-remove-gulp branch from 621e765 to 3a8583e Compare March 5, 2017 16:18

IgorMinar approved these changes Mar 5, 2017

View reviewed changes

IgorMinar added the action: merge The PR is ready for merge by the caretaker label Mar 5, 2017

IgorMinar merged commit 00fdcf4 into angular:master Mar 5, 2017

petebacondarwin deleted the aio-remove-gulp branch March 13, 2017 20:14

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019

aio: remove gulp tasks #14889

aio: remove gulp tasks #14889

Uh oh!

Conversation

petebacondarwin commented Mar 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IgorMinar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin Mar 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin commented Mar 3, 2017

Uh oh!

IgorMinar commented Mar 5, 2017

Uh oh!

angular-automatic-lock-bot bot commented Sep 11, 2019

Uh oh!

Uh oh!

petebacondarwin commented Mar 2, 2017 •

edited

Loading

petebacondarwin Mar 3, 2017 •

edited

Loading