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.

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

Add gulp option #103

Merged
merged 3 commits into from
Oct 15, 2015
Merged

Add gulp option #103

merged 3 commits into from
Oct 15, 2015

Conversation

jimmynotjim
Copy link
Contributor

This PR adds the option to start a project with Gulp instead of Grunt. It uses pretty much the same exact setup, just with a few tweaks.

Additions

  • Add Gulp templates to a /gulp directory
    • Tasks are broken into individual files
    • All tasks use a central config to set up directory structures

Removals

  • None

Changes

  • Adds askForBuildTool option during generation to allow user to choose their build tool
  • Moves Grunt template files to a /grunt directory

Testing

  • git checkout gulp option && npm link
  • create dummy project, outside this repo, mk test-project && cd_
  • run the generator in test-project yo cf making sure to choose Gulp as your build tool
  • after NPM & Bower complete, run gulp build
  • PROFIT!!!

Review

Screenshots

screen shot 2015-10-06 at 8 28 02 pm

screen shot 2015-10-06 at 8 28 40 pm

## Todos - None ## Checklist - [ ] Changes are limited to a single goal (no scope creep) - [ ] Code can be automatically merged (no conflicts) - [ ] Code follows the standards laid out in the [front end playbook](https://github.com/cfpb/front-end) - [ ] Passes all existing automated tests - [ ] New functions include new tests - [ ] New functions are documented (with a description, list of inputs, and expected output) - [ ] Placeholder code is flagged - [ ] Visually tested in supported browsers and devices - [ ] Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)

@jimmynotjim jimmynotjim mentioned this pull request Oct 7, 2015
@@ -14,6 +14,7 @@
},
"license": "<%= props.license %>",
"dependencies": { <% for(var i=0; i<components.length; i++) { %>
"<%= components[i].name %>": "<%= components[i].ver%>"<% if (i < components.length - 1) { %>,<% } %><% } %>
"<%= components[i].name %>": "<%= components[i].ver%>"<% if (i < components.length - 1) { %>,<% } %><% } %>,
"jquery.easing": "~1.3.0"
Copy link
Member

Choose a reason for hiding this comment

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

Why was jquery.easing added?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, good catch, I thought I'd removed everything browserify related.

@contolini
Copy link
Member

@jimmynotjim This is really exciting! Everything builds fine for me. I filed a PR against your fork with some smoke tests.

@jimmynotjim
Copy link
Contributor Author

Merged smoke tests and looking good.

@contolini
Copy link
Member

👍 S'all good here.

@jimmynotjim
Copy link
Contributor Author

Let me test this just once more before we merge it.

scripts: {
src: [
loc.lib + '/jquery/dist/jquery.js',
loc.lib + '/jquery.easing/js/jquery.easing.js',
Copy link
Member

Choose a reason for hiding this comment

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

Remove easing here if removed in #103 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, now I'm not sure, cause it was in the Gruntfile

jimmynotjim and others added 3 commits October 15, 2015 12:07
- Generator asks which build tool you prefer
- Installs Grunt or Gulp deps and template files based on your answer
@contolini
Copy link
Member

Good-to-go?

Let's hope so.

Mergin'.

contolini added a commit that referenced this pull request Oct 15, 2015
@contolini contolini merged commit 84aff41 into cfpb:master Oct 15, 2015
@ascott1
Copy link
Member

ascott1 commented Oct 15, 2015

YASSSSS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants