Skip to content

Conversation

@Swiip
Copy link
Owner

@Swiip Swiip commented Jan 2, 2015

This is a big refactoring PR, I'll try to list my goals:

Copy link
Contributor

Choose a reason for hiding this comment

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

I think dotfiles is gone so this can be removed.

Copy link
Owner Author

Choose a reason for hiding this comment

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

You're right but I think I'll keep this waiting for a further refactoring PR...

This one is too big already

@marani
Copy link
Contributor

marani commented Jan 2, 2015

It's neat I like the new gulp file templates.

@zckrs
Copy link
Collaborator

zckrs commented Jan 3, 2015

Huge PR but it's awesome

  • New structure of gulp tasks is more readable and easier to change for a new generate project
  • You fix lot of mistakes like multiple choices on htmlPreprocessor or the empty index.html
  • Nice idea for lodash templating but you should add a comment on process function to explain why

Are you sure _.cloneDeep() create a new object and not use reference in mocha test ? Each test should have a independent prompt

@marani
Copy link
Contributor

marani commented Jan 3, 2015

var characters = [
  { 'name': 'barney', 'age': 36 },
  { 'name': 'fred',   'age': 40 }
];

var deep = _.cloneDeep(characters);
deep[0] === characters[0];
// → false

as in lodash docs.

zckrs pushed a commit that referenced this pull request Jan 5, 2015
Reorganize gulpfiles & gulp tasks
@zckrs zckrs merged commit 00bdb92 into master Jan 5, 2015
@Toilal
Copy link
Contributor

Toilal commented Jan 5, 2015

👍 Great thanks !

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.

5 participants