Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in C-API with context compiler status #1038

Merged
merged 1 commit into from
Apr 2, 2015

Conversation

mgreter
Copy link
Contributor

@mgreter mgreter commented Apr 2, 2015

Fixes #1035 @am11, @rodneyrehm can you test if this fixes your issues?
This does NOT alter the existing public (beta) API, only internal changes!

@mgreter mgreter added this to the 3.2 milestone Apr 2, 2015
@mgreter mgreter self-assigned this Apr 2, 2015
@coveralls
Copy link

Coverage Status

Coverage increased (+0.39%) to 80.69% when pulling bb4777c on mgreter:bugfix/issue_1035 into fd7f20f on sass:master.

@rodneyrehm
Copy link
Contributor

Looks like this PR fixes the problem!


tested the following way:

# cd libsass
git checkout master && git fetch --prune && git pull --rebase
git remote add mgreter [email protected]:mgreter/libsass.git
git fetch mgreter
git merge mgreter/bugfix/issue_1035
# building my stuff

is there a simpler way to locally merge a PR manually adding all PRs to .git/config:

[remote "origin"]
  url = [email protected]:sass/libsass.git
  fetch = +refs/heads/*:refs/remotes/origin/*
  fetch = +refs/pull/*/head:refs/remotes/origin/pr/*

@drewwells
Copy link
Contributor

This is how I do it
https://help.github.com/articles/checking-out-pull-requests-locally/
On Thu, Apr 2, 2015 at 4:36 PM Rodney Rehm [email protected] wrote:

Looks like this PR fixes the problem!

tested the following way:

cd libsass

git checkout master && git fetch --prune && git pull --rebase
git remote add mgreter [email protected]:mgreter/libsass.git
git fetch mgreter
git merge mgreter/bugfix/issue_1035

building my stuff

is there a simpler way to locally merge a PR manually adding all PRs to
.git/config:

[remote "origin"]
url = [email protected]:sass/libsass.git
fetch = +refs/heads/:refs/remotes/origin/
fetch = +refs/pull//head:refs/remotes/origin/pr/


Reply to this email directly or view it on GitHub
#1038 (comment).

@mgreter
Copy link
Contributor Author

mgreter commented Apr 2, 2015

Thanks @rodneyrehm for the positive news! Again sorry for messing that one up! Going to merge this now, since I can confirm that node-sass does not segfault anymore (@am11 only one test failing) !

mgreter added a commit that referenced this pull request Apr 2, 2015
Fix bug in C-API with context compiler status
@mgreter mgreter merged commit 0b0a449 into sass:master Apr 2, 2015
@rodneyrehm
Copy link
Contributor

splendid, thank you for the quick fix!

@am11
Copy link
Contributor

am11 commented Apr 3, 2015

@mgreter, thanks! Should I wait for the next beta?

@rodneyrehm, I always tend to use pull with rebase. :)

Once I added the remotes after the first clone:

# first time
cd src/
git clone https://github.com/sass/libsass
cd libsass
git remote add mgreter-libsass https://github.com/mgreter/libsass
git remote add xzyfer-libsass https://github.com/xzyfer/libsass

later at any random point in time:

cd src/libsass
git pull --rebase mgreter-libsass feature-branch-name

(it fetches and rebase on your current branch and fallbacks to 3-way merge if there are conflicts which normally doesn't happen if you have added libsass as submodule like node-sass has)

@mgreter
Copy link
Contributor Author

mgreter commented Apr 3, 2015

@am11 use the latest master to update node-sass to. When you are ready to release a new beta for node-sass please, ping us and we will make another libsass beta!

@mgreter mgreter deleted the bugfix/issue_1035 branch April 6, 2015 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sass_compiler_get_last_import() does not provide previous path
5 participants