search

kamlekar / HTML-Skinning-Boilerplate

A Boilerplate/Scaffold to do HTML skinning faster for Websites.
15 stars 6 forks source link

Revert "Add variables for paths, added 'gulp-data' code" #28

Closed kamlekar closed 8 years ago

kamlekar commented 8 years ago

Reverts kamlekar/HTML-Skinning-Boilerplate#26

kamlekar commented 8 years ago

There were issues in PR https://github.com/kamlekar/HTML-Skinning-Boilerplate/pull/26.

ghost commented 8 years ago

What are issues?

kamlekar commented 8 years ago

Hi @rustemgareev I wasn't able to run gulp watch in CLI.

ghost commented 8 years ago

Have you added empty data.json? in site/data/

kamlekar commented 8 years ago

No, I haven't. Please send another PR with all the changes. And do we need gulp-data? if needed, please also send package.json change.

ghost commented 8 years ago

22 is PR with package.json

26 has descriptions that you should add data.json to "site/data/" folder.

kamlekar commented 8 years ago

But I don't think the error is related to empty json file or gulp-data. Is it working fine in your local? I mean did you run gulp watch?

ghost commented 8 years ago

What is written in console? No matter empty or filled - data.json should be in "site/data/" folder. This works fine to me.

kamlekar commented 8 years ago

Here is the screenshot of the issue:

http://i.imgur.com/bnuxfjl.png

kamlekar commented 8 years ago

Also, site directory is supposed to have only production ready (or final) code files. So, can we keep the empty data.json file outside of site folder?

ghost commented 8 years ago

This issue is unknown to me=) but I think it's with mispelling, I should check gulpfile.js. Also error will appear if data.json is not on correct directory (that happened to me sometimes).

As for data.json outside of site - I think no. For example if you want to use images in data.json, they should be on site, and data.json on site. But it's not problem - I think it's very useful, people can fill they site with data, not just static texts.

ghost commented 8 years ago

Yeah, problem is detected - one closing ) missed: 127 string .pipe(gulp.dest(basePath.site); shoulde be .pipe(gulp.dest(basePath.site));

kamlekar commented 8 years ago

Yup, the issue is that you missed one closing parenthesis in gulpfile.js :). Please check it from next time. Clearly, manual testing is taking more time. I need to assign myself to attach Travis asap. And also please add all related changes in one PR (I know you are struggling to add all in one PR, but I know you will figure it out soon). Please send me new PR will all changes.

kamlekar commented 8 years ago

@rustemgareev Also, I created new branch, pr-branch. Until we integrate travis we can't take risk on testing the new changes on working master branch. So, I added new branch. Please send PR to that branch.