search

JSRocksHQ / harmonic

The next static site generator
http://harmonicjs.com/
MIT License
282 stars 26 forks source link

Use rimraf instead of `rm -rf` #41

Closed UltCombo closed 10 years ago

UltCombo commented 10 years ago

Fixes Windows incompatibility and adds error handling to the Parser#clean task.

This fixes the issue reported here.

UltCombo commented 10 years ago

The diff looks a bit weird as I've converted the tabs to spaces in the clean function.

I'm unsure what would be the proper style for this commit, seeing as the style guide uses spaces for indentation and the function I'm currently editing is indented with tabs. Perhaps I should keep using tabs (or use spaces only in the edited lines) in order to generate a more readable diff?

//cc @jaydson

UltCombo commented 10 years ago

Or we could just wait until #39 is merged as well.

jaydson commented 10 years ago

Nice job @UltCombo ! Let's integrate the way it is. When we merge Leo's PR, we fix what we miss.

Well, i don't have Windows here, so i'll not run Harmonic on Windows. I'll just check if it doesn't break anything on Linux, ok?

UltCombo commented 10 years ago

@jaydson it shouldn't break anything, rimraf was made to be a platform-independent rm -rf.

This fixes the issue I've reported earlier, however, looks like I've just hit the issue you've reported earlier, so #32 is not fixed yet. I'll open a follow up PR for the next incompatibilities I find. =]

UltCombo commented 10 years ago

I've adjusted the PR and commit messages to reflect that this isn't a fix to #32 yet -- guess I need some more coffee in order to test things properly =]. Well, would you prefer me to group Windows incompatibilities patches into this PR or open a new PR for each issue?

jaydson commented 10 years ago

I'll merge this into master now. I think issue #32 must to be more explored, right?

UltCombo commented 10 years ago

@jaydson yup, I'll do some more exploring. :)