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

Build related fixes #609

Merged
merged 3 commits into from
Jan 8, 2015
Merged

Build related fixes #609

merged 3 commits into from
Jan 8, 2015

Conversation

am11
Copy link
Contributor

@am11 am11 commented Jan 8, 2015

Install: Improves code to get proxy.


Build: Use grep to filter test cases.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 902526f on am11:master into 520dd86 on sass:master.

@am11 am11 force-pushed the master branch 2 times, most recently from 61ceea2 to 3dae5be Compare January 8, 2015 01:29
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 3dae5be on am11:master into 520dd86 on sass:master.

@am11 am11 force-pushed the master branch 3 times, most recently from 7815296 to 9968f3e Compare January 8, 2015 01:50
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 9968f3e on am11:master into 520dd86 on sass:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling e579c06 on am11:master into 520dd86 on sass:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling e579c06 on am11:master into 520dd86 on sass:master.


function validateProxyUrl(url) {
if (/\n/.test(url)) {
url = url.replace(/\r?\n+/, '\n').split('\n')[8];
Copy link
Contributor

Choose a reason for hiding this comment

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

Because you're doing proxy.output.trim() before passing into validateProxyUrl I believe this needs to be [5] not [8]. I would just this be make it more resilient.

url = url.replace(/\r?\n+/, '\n').split('\n');
url = url[url.length - 3].trim(); // get the second last element.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The resilient approach looks promising. I rebased it: am11@6539e7b.

Thanks! 😸

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 6539e7b on am11:master into 520dd86 on sass:master.

* Uses different strategy to test binary.
* Test two cases and declare the binary OK.
Issue URL: sass#611.
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0591d14 on am11:master into 520dd86 on sass:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0591d14 on am11:master into 520dd86 on sass:master.

@am11 am11 changed the title Install: Improves code to get proxy. Build related fixes Jan 8, 2015
am11 added a commit that referenced this pull request Jan 8, 2015
@am11 am11 merged commit 1df37b4 into sass:master Jan 8, 2015
jiongle1 pushed a commit to scantist-ossops-m2/node-sass that referenced this pull request Apr 7, 2024
install newly required header in autotools build system
# 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