Add Gulp file to resolve issue #478 #820

Merged
merged 1 commit into from Dec 1, 2014

Projects

None yet

6 participants

@mkelley33
Contributor

These changes result in a Gulpfile having the same features as its Grunt counterpart. I wrote this code
in response to issue #478 I put the gulpfile in a directory named gulp-able along with README and package.json files.

@mkelley33 mkelley33 referenced this pull request Oct 5, 2014
Closed

add a gulp file #478

@fyockm
Contributor
fyockm commented Oct 6, 2014

@mkelley33 Looks great! Thanks for the hard work. Discussing with @liorkesos how best to integrate.

@marcellodesales

@mkelley33 That's awesome... thanks for providing that!!!

@liorkesos
Member

Hi Marcello. We don't want to rock the boat too much so this will be part
of us integrating yo into mean init and offering gulp as an option.
We have PRS for less and sass preconiler support and I want to alter the
grunt/gulp files based upon n the yo questions there as well.
If you could work with on brainstorming this it will be awesome.
Were going to discuss this later in the irc channel.
Thanks!
Lior
On Oct 6, 2014 9:37 PM, "Marcello de Sales" notifications@github.com
wrote:

@mkelley33 https://github.com/mkelley33 That's awesome... thanks for
providing that!!!


Reply to this email directly or view it on GitHub
#820 (comment).

@marcellodesales

@liorkesos Thanks a lot for the update... I want to find some time to be part of the discussion as I'm on at my work right now till 5pm PST... :S

I definitely would love to have the switch "mean init --with-build=gulp"... What's the time for the discussion, including timezone?

thanks!

@mkelley33
Contributor

@fyockm Thanks! It was my pleasure. I enjoy using both Grunt and Gulp so I had a lot of fun matching up the functionality.

I'd be happy to automate it in mean-cli via a command-line option to mean init if that helps.

Whatever you and @liorkesos decide, let me know if there's anything more I can contribute :)

@mkelley33
Contributor

@liorkesos I posted that last comment about the same time as yours so disregard the CLI bit. Yeoman and CSS preprocessing sounds excellent! I use LESS and Sass and have made gulp tasks for them before. Would you like me to add a LESS and / or Sass task to this PR?

@fyockm
Contributor
fyockm commented Oct 6, 2014

@mkelley33 sure, that would be great. There's an existing PR #811 for a LESS task with Grunt.

@mkelley33
Contributor

@fyockm Cool, I'll take a look at that tonight and match it up against that.

@marcellodesales

Sorry guys @fyockm, @mkelley33, I'm leaving my office now (10pm PST) and I might have missed the meeting... (was there one?) Anyway, thanks for supporting Gulp on this and I can help testing it!!! 👍

@mkelley33 mkelley33 Add gulp as grunt alternative in example project.
See README in the gulp-able directory for usage instructions.
f41539d
@mkelley33
Contributor

@liorkesos @fyockm I added the Gulp LESS task and squashed my commits. Let me know if I can help in any other way.

@liorkesos
Member

@drew can you try to add the yo "questions" to be triggered by mean init?
On Oct 9, 2014 11:16 AM, "Michaux Kelley" notifications@github.com wrote:

@liorkesos https://github.com/liorkesos @fyockm
https://github.com/fyockm I added the Gulp LESS task and squashed my
commits. Let me know if I can help in any other way.


Reply to this email directly or view it on GitHub
#820 (comment).

@mkelley33
Contributor

@liorkesos did you mean that last comment for @fyockm ?

@liorkesos
Member

yeah

On Thu, Oct 9, 2014 at 9:34 PM, Michaux Kelley notifications@github.com
wrote:

@liorkesos https://github.com/liorkesos did you mean that last comment
for @fyockm https://github.com/fyockm ?


Reply to this email directly or view it on GitHub
#820 (comment).

Lior Kesos - http://www.linnovate.net
Linnovate - Community Infrastructure Care
mail: lior@linnovate.net
office: +972 722500881
cell: +972 524305252
skype: liorkesos

@enkodellc
Contributor

@mkelley33 thanks for putting this together. I am testing it out now and it seems to work fairly well so far. I have one issue though that is either not working for me or I need some clarification. When I want to run production I use 'gulp env:production' and then 'gulp' it is supposed to update the default tasks with this line? FYI, I created a new task called move that moves some static files into the build directory.

if (process.env.NODE_ENV === 'production') {
  defaultTasks = ['clean', 'move', 'cssmin', 'uglify'];
}

Just curious if that was the process. I don't use Heroku but I do use a Windows server so I have a production folder so I updated the task below and it works great.

gulp.task('production', ['env:production', 'clean', 'move', 'cssmin', 'uglify']);
@enkodellc enkodellc commented on the diff Oct 28, 2014
gulp-able/gulpfile.js
+ process.env.NODE_ENV = 'development';
+});
+
+gulp.task('env:production', function () {
+ process.env.NODE_ENV = 'production';
+});
+
+gulp.task('karma:unit', function (done) {
+ karma.start({
+ configFile: __dirname + '/karma.conf.js',
+ singleRun: true
+ }, done);
+});
+
+gulp.task('loadTestSchema', function () {
+ require('server.js');
@enkodellc
enkodellc Oct 28, 2014 Contributor

I received an error here. Needed to update the path to './server.js'

gulp.task('loadTestSchema', function () {
  require('./server.js');
  require('meanio/lib/util').preload(__dirname + '/packages/**/server', 'model');
});
@mkelley33
Contributor

@enkodellc thank you for your changes and help testing this! I haven't deployed to a windows server in quite some time, and I think having a more general production task would be a great idea. The loadTestSchema change seems like a fine idea too. The addition of a move task seems reasonable too, but as far as any of these changes are concerned, I think @liorkesos and @fyockm might want to address these changes as well, yes? If they're amenable to the changes, then you could probably do something like submit a PR to my fork, which I would merge, and then update this PR. Does that work?

@enkodellc
Contributor

I am not sure the mean repo needs the move task... My project just has some sounds and images that are required to be moved to the bower_components folder so that they are still relative to the build folder.

gulp.task('move', function(){
  // the base option sets the relative root for the set of files,
  // preserving the folder structure 
  gulp.src('./packages/system/public/assets/img/**', { base: './packages/system/public/assets/img/' })
    .pipe(gulp.dest('bower_components/img'));
  gulp.src('./packages/system/public/assets/sound/**', { base: './packages/system/public/assets/sound/' })
    .pipe(gulp.dest('bower_components/sound'));
});

Did you not get an error when running the "gulp test" with the "server.js" path?? I was so I had to update it to "./server.js"

Whatever approach you want to take is fine by me. Right now I haven't spent much time testing any speed changes... so far I feel it might be negligible but I think it has helped with some of the livereload / aggregate issues that are mentioned in #529.

Either way I am currently using gulp exclusively and making a lot of changes and testing it out pretty hard. Will know more after uploading to my server and some more testing. Great work.

@liorkesos liorkesos referenced this pull request Nov 25, 2014
Closed

Extend mean init #914

@fyockm
Contributor
fyockm commented Nov 26, 2014

@mkelley33 sorry for the long delay. We sincerely appreciate your contribution and plan to get this in place very soon.

@enkodellc perhaps you'll be willing to review once we get the gulp option in place?

@enkodellc
Contributor

Definitely. I have been using the gulp almost exclusively for the past month. It runs faster to do the tests as mocha and karma run in parallel. I did not see a giant performance increase in the standard default run but it seems to be working better than grunt during live reloads... though those issues might have been taken care of during other PR's. Props to @mkelley33

@fyockm
Contributor
fyockm commented Nov 26, 2014

@mkelley33 I like the approach of putting gulp in a separate folder, but perhaps "gulp" would be better than "gulp-able"?
Another thought - I dislike needing to maintain 2 separate package.json files. Might be better to have a json file with only the gulp-* packages? Call it gulp.json, and we could have a similar grunt.json. Then during mean init we could simply merge that file with package.json.

{
  "dependencies": {...}, // if any?
  "devDependencies": {
    "gulp-rimraf": "^0.1.1",
    "jshint-stylish": "^1.0.0",
    "gulp-load-plugins": "^0.7.0",
    "gulp-cssmin": "^0.1.6",
    "gulp-csslint": "^0.1.5",
    "del": "^0.1.3",
    "gulp-livereload": "^2.1.1",
    "gulp-jshint": "^1.8.5",
    "gulp-nodemon": "^1.0.4",
    "gulp-uglify": "^1.0.1",
    "gulp-concat": "^2.4.1",
    "gulp-mocha": "^1.1.0",
    "gulp": "^3.8.8",
    "gulp-util": "^3.0.1",
    "through": "^2.3.6",
    "gulp-less": "^1.3.6"
  }
}
@liorkesos
Member

One more f the issues we need to try to address is to "clean up" the root
Dir. So if we had a subdir - "tools" or something we could place those
jsons there .
Package/bower/mean.json should probably stay in the root but Travis , a
Dockerfile and others might migrate there (if these system don't assume the
file is in the rootdir and that is not changeable)
Just my 2 cents
Lior
On Nov 26, 2014 5:54 AM, "Drew Fyock" notifications@github.com wrote:

@mkelley33 https://github.com/mkelley33 I like the approach of putting
gulp in a separate folder, but perhaps "gulp" would be better than
"gulp-able"?
Another thought - I dislike needing to maintain 2 separate package.json
files. Might be better to have a json file with only the gulp-* packages?
Call it gulp.json, and we could have a similar grunt.json. Then during mean
init we could simply merge that file with package.json.

{
"dependencies": {...}, // if any?
"devDependencies": {
"gulp-rimraf": "^0.1.1",
"jshint-stylish": "^1.0.0",
"gulp-load-plugins": "^0.7.0",
"gulp-cssmin": "^0.1.6",
"gulp-csslint": "^0.1.5",
"del": "^0.1.3",
"gulp-livereload": "^2.1.1",
"gulp-jshint": "^1.8.5",
"gulp-nodemon": "^1.0.4",
"gulp-uglify": "^1.0.1",
"gulp-concat": "^2.4.1",
"gulp-mocha": "^1.1.0",
"gulp": "^3.8.8",
"gulp-util": "^3.0.1",
"through": "^2.3.6",
"gulp-less": "^1.3.6"
}
}


Reply to this email directly or view it on GitHub
#820 (comment).

@fyockm
Contributor
fyockm commented Nov 26, 2014

@liorkesos I like that idea - tools. Once we publish 4.1, let's merge this PR to give @mkelley33 credit, and we can manipulate things from there.

@liorkesos
Member

I just merged the aggregation and technically tagged 0.4.1 about an hour ago.
Go for it!

@pratik60
Member

Have you guys all thoroughly tested 0.4.1? It works with existing apps, and
everything?

I couldn't get the time :-(

@liorkesos
Member

We've tested it and it passed @andrija-hers @ellman @liorkesos and others, we don't have an "existing site" we test on it might be a good idea to add such an instance before for the next versions.

@mkelley33
Contributor

@fyockm yes, I agree that the directory name "gulp" would be better, because it'd be more suited to an international audience and just make sense when reading through the code.

I also agree that having to maintain two separate package files would be bad.

I recently got a new job, so I'll need to ask my employer before making any further contributions or changes. Once that is done, I can make the changes, or feel free to make the change yourself so
others don't have to wait.

@fyockm
Contributor
fyockm commented Nov 26, 2014

@mkelley33 Congrats on the new job! I'll let the dust settle on 4.1 for a day or so, then merge this PR and massage it a bit. Thanks again for the contribution and apologies for the delay.

@mkelley33
Contributor

@fyockm Thank you! I'm happy to contribute. It was my pleasure. As for the delay: not a problem, I totally understand how work, family, and other priorities can delay merging a PR and many other daily tasks :) Please let me know if I can be of any further assistance or contribute in any other ways.

@fyockm fyockm merged commit f41539d into linnovate:master Dec 1, 2014

2 checks passed

clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/travis-ci The Travis CI build passed
Details
@marcellodesales

@mkelley33 Thanks for providing this!!!

@mkelley33 mkelley33 deleted the mkelley33:gulp-able branch Jul 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment