Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP exclude browserSync and connect-history-api-fallback #95

Merged
merged 14 commits into from
Jun 8, 2016
Merged

WIP exclude browserSync and connect-history-api-fallback #95

merged 14 commits into from
Jun 8, 2016

Conversation

darxx
Copy link
Contributor

@darxx darxx commented Jun 7, 2016 •
edited by antonybudianto7
Loading

I what to separate browser-sync and have problems running gulp build --exclude-source --prod.
This is work in progress. Hope together we will succeed.

Edit by antonybudianto:
TODOS

Closes #94

antonybudianto
antonybudianto reviewed Jun 7, 2016
View reviewed changes
gulp.config.js

module.exports = function () {
var root = '';
var src = root + 'src/';
var translations = src + 'i18n/*.json';
Copy link
Owner

Choose a reason for hiding this comment

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

this one is not related right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes is not related. This is copy paste from working example that has localization added. If this problem related to node module separation will be solved. I can make another pull request and add localization and related unit tests.

antonybudianto
antonybudianto reviewed Jun 7, 2016
View reviewed changes
gulp.config.js
"/src": "src"
}

if (global.env === 'dev')
Copy link
Owner

Choose a reason for hiding this comment

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

Please store mode 'dev' as constant, so it's not hardcoded and expose it to config var

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added
global.env = 'dev';
global.env = argv.prod ? 'prod': 'dev';

To gulp.file.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And I know that build is failing. I can`t find why.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

C:\dev\angular2-starter>gulp build --exclude-source --prod
[16:44:39] Using gulpfile C:\dev\angular2-starter\gulpfile.js
[16:44:39] Starting 'build'...
[16:44:39] Starting 'clean-report'...
[16:44:39] Starting 'clean-ts'...
[16:44:39] Finished 'clean-report' after 6.99 ms
[16:44:39] Finished 'clean-ts' after 24 ms
[16:44:39] Starting 'tsc'...
[16:44:43] Finished 'tsc' after 4.74 s
[16:44:43] Starting 'unit-test'...
07 06 2016 16:44:49.221:INFO [karma]: Karma v0.13.22 server started at http://localhost:9876/
07 06 2016 16:44:49.229:INFO [launcher]: Starting browser PhantomJS
07 06 2016 16:44:50.816:INFO [PhantomJS 2.1.1 (Windows 8 0.0.0)]: Connected on socket /#T-pSbDyh595rPdjEAAAA with id 68686206
07 06 2016 16:44:53.273:WARN [web-server]: 404: /base/node_modules/karma-ie-launcher/test/launcher.spec
PhantomJS 2.1.1 (Windows 8 0.0.0) ERROR
Error: XHR error (404 Not Found) loading http://localhost:9876/node_modules/karma-ie-launcher/test/launcher.spec
Error loading http://localhost:9876/node_modules/karma-ie-launcher/test/launcher.spec

07 06 2016 16:44:53.337:WARN [web-server]: 404: /base/node_modules/karma-phantomjs-launcher/test/add.spec

Copy link
Owner

Choose a reason for hiding this comment

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

I'm tracking the error right now

@darxx
Copy link
Contributor Author

darxx commented Jun 7, 2016 •
edited
Loading

Please store mode 'dev' as constant, so it's not hardcoded and expose it to config var

I need predefined hard coded environment.

@antonybudianto
Copy link
Owner

antonybudianto commented Jun 7, 2016 •
edited
Loading

please update .travis.yml build script to

gulp build --exclude-source --env=prod

antonybudianto
antonybudianto reviewed Jun 7, 2016
View reviewed changes
gulpfile.js
@@ -1,3 +1,8 @@
var argv = require('yargs').argv;

global.env = 'dev';
Copy link
Owner

@antonybudianto antonybudianto Jun 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

not needed, remove this

@antonybudianto
Copy link
Owner

antonybudianto commented Jun 7, 2016 •
edited
Loading

@darxx , looks like I need to refactor the config since the behavior changes. Please wait, it should fix the error
After #96 merged, please rebase to latest master

@darxx
Copy link
Contributor Author

darxx commented Jun 7, 2016

Hope changes will be useful to everyone. Ok I will wait. After I will fork this project again. Im not spending much time on github. Dont know how to make rebase on forked project.

@antonybudianto
Copy link
Owner

you can also pull latest master and merge it to your branch

antonybudianto
antonybudianto reviewed Jun 7, 2016
View reviewed changes
.idea/jsLibraryMappings.xml
<component name="JavaScriptLibraryMappings">
<file url="file://$PROJECT_DIR$" libraries="{angular2-starter node_modules}" />
</component>
</project>
Copy link
Owner

Choose a reason for hiding this comment

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

remove this file

@coveralls
Copy link

coveralls commented Jun 7, 2016 •
edited
Loading

Coverage Status

Changes Unknown when pulling 466d412 on darxx:master into * on antonybudianto:master*.

antonybudianto
antonybudianto reviewed Jun 7, 2016
View reviewed changes
gulpfile.js

global.env = argv.env || 'dev';
console.log(global.env);
Copy link
Owner

Choose a reason for hiding this comment

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

remove this

@coveralls
Copy link

coveralls commented Jun 7, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling 0e2c5fe on darxx:master into ef02ab4 on antonybudianto:master.

@coveralls
Copy link

coveralls commented Jun 7, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling 0e2c5fe on darxx:master into ef02ab4 on antonybudianto:master.

@coveralls
Copy link

coveralls commented Jun 7, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling d16583d on darxx:master into ef02ab4 on antonybudianto:master.

@coveralls
Copy link

coveralls commented Jun 7, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling 6746eb9 on darxx:master into ef02ab4 on antonybudianto:master.

antonybudianto7
antonybudianto7 reviewed Jun 8, 2016
View reviewed changes
gulp.config.js
var argv = require('yargs').argv;

var environment = argv.env || 'dev';
console.log(environment);
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this

Darius Laurinčikas added 2 commits June 8, 2016 09:24
@coveralls
Copy link

coveralls commented Jun 8, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling 08cdf17 on darxx:master into ef02ab4 on antonybudianto:master.

@coveralls
Copy link

coveralls commented Jun 8, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling 08cdf17 on darxx:master into ef02ab4 on antonybudianto:master.

antonybudianto7
antonybudianto7 reviewed Jun 8, 2016
View reviewed changes
package.json
"connect-history-api-fallback": "^1.2.0",
"del": "~2.2.0",
"zone.js": "^0.6.12",
"del": "~2.2.0",
Copy link
Collaborator

@antonybudianto7 antonybudianto7 Jun 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

please remove the tab/additional spaces here

@antonybudianto7
Copy link
Collaborator

the rest LGTM, will check it out tonight

@coveralls
Copy link

coveralls commented Jun 8, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling 1757ab5 on darxx:master into ef02ab4 on antonybudianto:master.

@antonybudianto7 antonybudianto7 added this to the RC 3 milestone Jun 8, 2016
@antonybudianto antonybudianto merged commit 93fdb93 into antonybudianto:master Jun 8, 2016
@antonybudianto
Copy link
Owner

antonybudianto commented Jun 8, 2016 •
edited
Loading

Thanks for the PR! @darxx
🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
RC 3
Development

Successfully merging this pull request may close these issues.
4 participants
@darxx @antonybudianto @coveralls @antonybudianto7