Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Output directory is created if it doesn't exists. #593

Merged
merged 1 commit into from
Jan 5, 2015

Conversation

SebastianOsuna
Copy link
Contributor

Pull Request for #574 .

Changes are on 'output-directory' branch.

:)

});



Copy link
Contributor

Choose a reason for hiding this comment

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

Lets don't add extra linefeeds. Between consecutive blocks one blank line is enough.

@am11
Copy link
Contributor

am11 commented Dec 31, 2014

LGTM 👍

Thanks @SebastianOsuna! ⚡

@SebastianOsuna
Copy link
Contributor Author

@am11 No problem. I hope it helps.

Do you need me to change the line you commented?

@am11
Copy link
Contributor

am11 commented Dec 31, 2014

Yes and rebase. That would nice! :)

@kevva
Copy link
Member

kevva commented Jan 1, 2015

-1. Just replace fs with fs-extra and use the fs.outputFile() function instead of fs.writeFile().

@am11
Copy link
Contributor

am11 commented Jan 1, 2015

I don't think we need extra dependency (which further depends on three more dependencies) for a little chore like this.

@kevva
Copy link
Member

kevva commented Jan 1, 2015

Well, this needs some rewriting then. Code looks horrible. For instance, we don't need to check if the folder exists or not, mkdirp takes care of that.

}
// Check if output directory exists
var dest_dir = path.dirname(path.normalize(options.dest));
var write_out = function() {
Copy link
Member

Choose a reason for hiding this comment

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

Functions are written using camelCase like function writeOut() and not using var. But we don't need this function anyway.

@am11
Copy link
Contributor

am11 commented Jan 2, 2015

@SebastianOsuna, we are readying for vNext, could you make those changes and rebase soon?

@SebastianOsuna
Copy link
Contributor Author

Fixes done. I'm not sure if the rebase was successful, I'm not an expert with git yet, sorry.

@am11
Copy link
Contributor

am11 commented Jan 3, 2015

@SebastianOsuna,

git pull --rebase
git rebase -i HEAD~5

# the editor will open
# change the word 'pick' to 'f', for each
# uncommented line *after* the one with text:
# "Output directory is created if it doesn't exists. "
# save and close the editor

# now forcefully rewrite the history:
git push -f

That's it! :)

@SebastianOsuna
Copy link
Contributor Author

Rebase done with the commands provided. Sorry for the delay.

@am11
Copy link
Contributor

am11 commented Jan 5, 2015

@SebastianOsuna, the outcome should be one commit. But right now you have 8 commits.

So lets try this:

git remote add node-sass https://github.com/sass/node-sass
git pull --rebase node-sass master
git push -f

This will clean those other commits your PR is showing.

Then we will rebase in interactive mode (later, once you rebase against node-sass master).

@SebastianOsuna
Copy link
Contributor Author

Ok, did that but still 2 commits and both duplicated :(

@am11
Copy link
Contributor

am11 commented Jan 5, 2015

That's ok. Now you can run: git rebase -i HEAD~4. And then in editor, except for the first line, change pick to f for the next three lines, save and close the editor. Finally, git push -f, to rewrite the history.

@SebastianOsuna
Copy link
Contributor Author

Sorry, but I can't seem to make it work. Each time I try the fixup, the log doesn't change. I even tried with the squash option, but no success.

@am11
Copy link
Contributor

am11 commented Jan 5, 2015

If you are on Windows, you would probably get a notepad opened after you do git rebase -i HEAD~4 with the following:

pick 099e501 Output directory is created if it doesn't exists.
pick b180fb5 Output directory is created if it doesn't exists.  
pick c2d56dc Code styling fixes.
pick 91f2012 Code styling fixes.

Change it to:

pick 099e501 Output directory is created if it doesn't exists.
f b180fb5 Output directory is created if it doesn't exists.  
f c2d56dc Code styling fixes.
f 91f2012 Code styling fixes.

Ctrl+S and close notepad. Finally run git push -f.

Does this work?

@SebastianOsuna
Copy link
Contributor Author

Was closing vim with the wrong command, sorry.

am11 added a commit that referenced this pull request Jan 5, 2015
Output directory is created if it doesn't exists.
@am11 am11 merged commit f7a2ee5 into sass:master Jan 5, 2015
@am11
Copy link
Contributor

am11 commented Jan 5, 2015

Perfect. Thanks for your patience and contribution! 👍

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants