cfpb / generator-cf

Yeoman generator for Capital Framework
http://cfpb.github.io/capital-framework/getting-started/
Creative Commons Zero v1.0 Universal
9 stars 13 forks source link

Update generator logic to support CF v1.x.x #83

Closed contolini closed 9 years ago

contolini commented 9 years ago

Uses the new @import technique instead of concatenating the cf- files.

Added

This PR has some strong opinions. Notably, the removal of the grunt bower task and related exportsOverride section of bower.json. That grunt task has always felt ugly and esoteric to me. It's not easy to determine what it does without studying the task's docs.

The result is that the directories in src/vendor are now populated with the full contents of the dependencies' repos instead of just their "main" files. I don't view this as a problem because it's Bower's default behavior and it's how other managers like npm do it. I know others liked that task, though, so please voice any concerns.

ascott1 commented 9 years ago

Checks author's email to determine if they're a CFPB employee and uncomments licensed-fonts.css if they are.

magic

anselmbradford commented 9 years ago

This PR has some strong opinions. Notably, the removal of the grunt bower task and related exportsOverride section of bower.json. That grunt task has always felt ugly and esoteric to me. It's not easy to determine what it does without studying the task's docs.

Yay! I too felt this task is hard to follow and it's probably best to move away from a plugin that has been looking for a maintainer for nearly a year. I can test later, but I just wanted to check on two things related to this task:

contolini commented 9 years ago

@anselmbradford Cool, good to know someone else wasn't a fan of that task!

box-sizing-polyfill is installed by cf-grid, which expects it to be in /static/vendor/, when the Gruntfile moves the box-sizing-polyfill here will it end up in /dist/static/ or /dist/static/vendor/? That is, does the copy task include the directory it found the file in when it performs the copy or will it just move the file without the underlying structure?

The copy task includes the directory so we should be good here.

screen shot 2015-06-03 at 2 47 38 pm

In flapjack we still had to move html5shiv because that's not concatenated with the rest of the JS since it needs to be loaded before other scripts load. Is it a problem that it's not being moved anymore?

Hmmm, I noticed that bower wasn't downloading html5shiv so I deleted that task. It's possible I erroneously deleted that dependency from one of the CF components when I did all the v1 changes. Do you know which component requires it?

anselmbradford commented 9 years ago

Hmmm, I noticed that bower wasn't downloading html5shiv so I deleted that task. It's possible I erroneously deleted that dependency from one of the CF components when I did all the v1 changes. Do you know which component requires it?

hm, I'm actually not sure that CF requires it, so this may be fine. It could just be that flapjack was using it. If there are any semantic HTML elements in use in CF they would require it I suspect.

sephcoster commented 9 years ago

I know you didn't ask - but, changes look good to me. Any changes that result in a passing build after generation while not being prescriptive about included things above and beyond what's needed seems good to me! :+1:

contolini commented 9 years ago

Cool, I anticipate patches coming after this but let's kick the tires.

contolini commented 9 years ago

Tagged and published. Run yo and select "Update your generators".