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

gulp fixes and reverts #542

Merged
merged 4 commits into from
Sep 14, 2015
Merged

gulp fixes and reverts #542

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9b4ff41
gulp specs to prevert futher fubars
nmccready Sep 14, 2015
1d2b8ac
new gulp specs reordered to not race
nmccready Sep 14, 2015
c0f3645
gulp fixed
nmccready Sep 14, 2015
f8ac745
watch removed from series to parallel
nmccready Sep 14, 2015
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
6 changes: 0 additions & 6 deletions backend/config/express.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,6 @@ app.use helmet.nocache()
# ensure all assets and data are compressed - above static
app.use compress()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Killed dep as @zacronos mentioned.. this was not needed

# setting the favicon and static folder
# NOTE TODO: THIS IS THE ONLY DEPENDENCY THAT EXPRESS HAS on Assets to be compiled prior
# THEREFORE it would be nice to set this setting reactivley at a later time to speed up builds.
app.use favicon "#{config.FRONTEND_ASSETS_PATH}/assets/favicons/favicon.ico"
app.use serveStatic config.FRONTEND_ASSETS_PATH

# cookie parser - above session
app.use cookieParser config.SESSION.secret

Expand Down
6 changes: 4 additions & 2 deletions gulp/tasks/default.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ gulp = require 'gulp'
].forEach (dep) ->
# console.log 'requiring', dep
require dep
#help = require('gulp-help')(gulp)

gulp.task 'developNoSpec', gulp.series 'clean', 'otherAssets', gulp.parallel('express', 'watch')
#this allows `gulp help` task to work which will display all taks via CLI so yes it is used
Copy link
Contributor Author

Choose a reason for hiding this comment

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

notes on gulp-help
chmontgomery/gulp-help#31

# help = require('gulp-help')(gulp) #BROKEN IN GULP 4

gulp.task 'developNoSpec', gulp.series 'clean', gulp.parallel('angular', 'angularAdmin', 'otherAssets', 'express'), 'watch'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually I think I can parallelize this watch now. Ill change and test, dont merge yet on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@realtyjustin your main problem was here as angular and angularAdmin needed to be called explicitly and not in the watches themselves. As I said I don't know how this was working to begin with. The only thing I can think of was that it was due to PR changes via pr suggestions. (Which would have back tracked what was working). IE I think when I removed the complicated function which built the series of parallels it broke the above.


#note specs must come after watch since browserifyWatch also builds scripts
gulp.task 'develop', gulp.series 'developNoSpec', 'spec'
Expand Down
25 changes: 21 additions & 4 deletions gulp/tasks/markup.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ plumber = require 'gulp-plumber'

$ = require('gulp-load-plugins')()

_testCb = null

markup = (app) ->
_testCb() if _testCb

gulp.src paths[app].jade
.pipe plumber()
.pipe $.consolidate 'jade',
Expand All @@ -29,16 +33,29 @@ markup = (app) ->
markupImpl = -> markup 'map'
markupAdminImpl = -> markup 'admin'

watchImpl = ->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Watches follow gulp documentation.

gulp.watch paths.map.jade, markupImpl

watchAdminImpl = ->
gulp.watch paths.admin.jade, markupAdminImpl

gulp.task 'markup', markupImpl

gulp.task 'markupWatch', (done) ->
gulp.watch paths.map.jade
markupImpl()
watchImpl()
done()

gulp.task 'markupAdmin', markupAdminImpl

gulp.task 'markupWatchAdmin', (done) ->
gulp.watch paths.admin.jade
markupAdminImpl()
watchAdminImpl()
done()

module.exports =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exports are for testing only.

###
For intent and purposes these exports are for testing only
###
watchImpl: watchImpl
watchAdminImpl:watchAdminImpl
setTestCb: (cb) ->
_testCb = cb
6 changes: 4 additions & 2 deletions gulp/tasks/spec.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ require './otherAssets'
require './karma'
require './mocha'

gulp.task 'spec', gulp.parallel 'commonSpec', 'backendSpec', 'gulpSpec', 'frontendSpec'
gulp.task 'spec', gulp.series gulp.parallel('commonSpec', 'backendSpec', 'frontendSpec'), 'gulpSpec'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

gulpSpec must come last or first out of all specs or it will fail.


gulp.task 'rebuildSpec', gulp.series gulp.parallel('commonSpec', 'backendSpec', 'gulpSpec'), gulp.parallel('otherAssets', 'angular', 'angularAdmin'), 'frontendSpec'
gulp.task 'rebuildSpec', gulp.series(
gulp.parallel('commonSpec', 'backendSpec'), 'gulpSpec'
, gulp.parallel('otherAssets', 'angular', 'angularAdmin'), 'frontendSpec')

gulp.task 'rspec', gulp.series 'rebuildSpec'

Expand Down
40 changes: 29 additions & 11 deletions gulp/tasks/styles.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ gulp = require 'gulp'
conf = require './conf'
$ = require('gulp-load-plugins')()

_testCb = null

styles = (src) ->
_testCb() if _testCb

gulp.src [
src.less
src.styles
Expand Down Expand Up @@ -40,27 +44,41 @@ stylesImpl = ->
stylesAdminImpl = ->
styles paths.admin

gulp.task 'styles', stylesImpl

gulp.task 'stylesWatch', (done) ->
watchImpl = ->
gulp.watch [
paths.map.less
paths.map.styles
paths.map.stylus
]
stylesImpl()
done()

gulp.task 'stylesAdmin', stylesAdminImpl
], stylesImpl

gulp.task 'stylesWatchAdmin', (done) ->
watchAdminImpl = ->
gulp.watch [
paths.map.less
paths.map.styles
paths.map.stylus
paths.admin.less
paths.admin.styles
paths.admin.stylus
]
stylesAdminImpl()
], stylesAdminImpl

gulp.task 'styles', stylesImpl

gulp.task 'stylesWatch', (done) ->
watchImpl()
done()

gulp.task 'stylesAdmin', stylesAdminImpl

gulp.task 'stylesWatchAdmin', (done) ->
watchAdminImpl()
done()


module.exports =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

like markup Exports are for testing only.

###
For intent and purposes these exports are for testing only
###
watchImpl: watchImpl
watchAdminImpl:watchAdminImpl
setTestCb: (cb) ->
_testCb = cb
10 changes: 1 addition & 9 deletions gulp/tasks/watch.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,5 @@ gulp.task 'watch_all_front', gulp.parallel 'angularWatch', 'angularWatchAdmin'
gulp.task 'watch', gulp.series 'watch_all_front', (done) ->
gulp.watch ['gulp/**/*.coffee','spec/gulp/**/*.coffee', specCommon], gulp.series 'gulpSpec'
gulp.watch ['backend/**/*.coffee', 'spec/backend/**/*.coffee', specCommon], gulp.series 'backendSpec'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

paths.destFull.scripts + '/map.bundle.js',      
-    paths.destFull.scripts + '/map.templates.js',      
-    paths.destFull.scripts + '/admin.bundle.js',       
-    paths.destFull.scripts + '/admin.templates.js',

because this is an artifact and it should not be watched. This was causing extra build times and and re-kicking the build as files were being moved and shuffled by gulp. We don't need to watch the artifacts we need to watch the frontend stuff which we are already doing via watch_all_front .

Eliminating this also eliminates the need to have watch run at a specific time. Heck it could even be added to be prior to clean via parallel.

Copy link
Contributor

Choose a reason for hiding this comment

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

This was my bad, I listed the artifacts so when they were rebuilt, Karma would run specs. But looking at the Karma docs it looks like it watches everything configured within files by default (http://karma-runner.github.io/0.8/config/files.html). That does make me wonder why Karma wasn't triggering twice each time though.. am I understanding this correctly @nmccready ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The watch inside karma itself is off so all watching is done by gulp. This is done to avoid complications.

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case, will specs still run on a rebuild?

gulp.watch [
paths.destFull.scripts + '/map.bundle.js',
paths.destFull.scripts + '/map.templates.js',
paths.destFull.scripts + '/admin.bundle.js',
paths.destFull.scripts + '/admin.templates.js',
'spec/app/**/*.coffee',
'spec/admin/**/*.coffee',
specCommon
], gulp.series 'frontendSpec'
gulp.watch ['spec/app/**/*.coffee','spec/admin/**/*.coffee', specCommon], gulp.series 'frontendSpec'
done()
34 changes: 34 additions & 0 deletions spec/gulp/markup.spec.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
subject = require '../../gulp/tasks/markup'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enforce via specs to find errors on gulp mods.

_testCb = null

describe 'gulp markup', ->
beforeEach ->
_testCb = null
subject.setTestCb null

afterEach ->
_testCb = null
subject.setTestCb null

describe 'watch', ->

['map', 'admin'].map (name) ->
testName:name
camelCaseName: if name is 'map' then '' else name.toInitCaps()
. forEach (names) ->

describe names.testName, ->
it 'SHOULD ONLY WATCH', (done) ->
called = false

_testCb = ->
called = true

subject.setTestCb(_testCb)
watcher = subject["watch#{names.camelCaseName}Impl"]()

setTimeout ->
called.should.equal(false)
watcher.end()
done()
, 200
34 changes: 34 additions & 0 deletions spec/gulp/styles.spec.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
subject = require '../../gulp/tasks/styles'
_testCb = null

describe 'gulp styles', ->
beforeEach ->
_testCb = null
subject.setTestCb null

afterEach ->
_testCb = null
subject.setTestCb null

describe 'watch', ->

['map', 'admin'].map (name) ->
testName:name
camelCaseName: if name is 'map' then '' else name.toInitCaps()
. forEach (names) ->

describe names.testName, ->
it 'SHOULD ONLY WATCH', (done) ->
called = false

stylesTestCb = ->
called = true

subject.setTestCb(_testCb)
watcher = subject["watch#{names.camelCaseName}Impl"]()

setTimeout ->
called.should.equal(false)
watcher.end()
done()
, 200