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

Reconsider the process.env.SKIP_SASS_BINARY_DOWNLOAD_FOR_CI #1453

Closed
saper opened this issue Apr 19, 2016 · 2 comments
Assignees
Milestone

Comments

@saper
Copy link
Member

@saper saper commented Apr 19, 2016

I've always had some doubts about this variable but during the v3.5.0 release process (#1451) it became obvious to me: why have a separate variable only to stop the download, if another (SASS_FORCE_BUILD or -f flag) is used to force the build anyway?

So, we have those effects:

SKIP_SASS_BINARY_DOWNLOAD_FOR_CI=1 and SASS_FORCE_BUILD=0 : no download, the build will be started due to empty vendor directory if the binary hasn't been planted there otherwise (manual install)

SKIP_SASS_BINARY_DOWNLOAD_FOR_CI=0 and SASS_FORCE_BUILD=1: download occurs, but the build proceeds anyway, overwriting the downloaded file (if successful).

Maybe we should use only SASS_FORCE_BUILD and drop the unofficial SKIP_SASS_BINARY_DOWNLOAD_FOR_CI ?

@xzyfer

This comment has been minimized.

Copy link
Contributor

@xzyfer xzyfer commented Apr 27, 2016

There's an interesting use case for SKIP_SASS_BINARY_DOWNLOAD_FOR_CI documented in #1037 (comment).

Maybe we should use only SASS_FORCE_BUILD and drop the unofficial SKIP_SASS_BINARY_DOWNLOAD_FOR_CI ?

I'd be curious to see if this has any side effects. I predict slightly slower builds which isn't a bug deal.

@nschonni nschonni added this to the 4.0 milestone Sep 19, 2016
xzyfer added a commit to xzyfer/node-sass that referenced this issue Sep 26, 2016
This flag means we skip a bunch of code paths in CI which is not
ideal. Since we tell people to use `npm rebuild` we should eat our
own dog food.

With this patch we do an install, then do a rebuild hitting all the
major install code paths.

Fixes sass#1453
xzyfer added a commit to xzyfer/node-sass that referenced this issue Sep 26, 2016
This flag means we skip a bunch of code paths in CI which is not
ideal. Since we tell people to use `npm rebuild` we should eat our
own dog food.

With this patch we do an install, then do a rebuild hitting all the
major install code paths.

Fixes sass#1453
xzyfer added a commit to xzyfer/node-sass that referenced this issue Nov 8, 2016
This flag means we skip a bunch of code paths in CI which is not
ideal. Since we tell people to use `npm rebuild` we should eat our
own dog food.

With this patch we do an install, then do a rebuild hitting all the
major install code paths.

Fixes sass#1453
@xzyfer xzyfer modified the milestones: 4.0, 5.0 Jan 12, 2017
xzyfer added a commit that referenced this issue Nov 7, 2019
This flag means we skip a bunch of code paths in CI which is not
ideal. Since we tell people to use `npm rebuild` we should eat our
own dog food.

With this patch we do an install, then do a rebuild hitting all the
major install code paths.

Fixes #1453
xzyfer added a commit to xzyfer/node-sass that referenced this issue Nov 7, 2019
This flag means we skip a bunch of code paths in CI which is not
ideal. Since we tell people to use `npm rebuild` we should eat our
own dog food.

With this patch we do an install, then do a rebuild hitting all the
major install code paths.

Fixes sass#1453
xzyfer added a commit to xzyfer/node-sass that referenced this issue Nov 7, 2019
This flag means we skip a bunch of code paths in CI which is not
ideal. Since we tell people to use `npm rebuild` we should eat our
own dog food.

With this patch we do an install, then do a rebuild hitting all the
major install code paths.

Fixes sass#1453
xzyfer added a commit to xzyfer/node-sass that referenced this issue Nov 7, 2019
This flag means we skip a bunch of code paths in CI which is not
ideal. Since we tell people to use `npm rebuild` we should eat our
own dog food.

With this patch we do an install, then do a rebuild hitting all the
major install code paths.

Fixes sass#1453
xzyfer added a commit to xzyfer/node-sass that referenced this issue Nov 7, 2019
This flag means we skip a bunch of code paths in CI which is not
ideal. Since we tell people to use `npm rebuild` we should eat our
own dog food.

With this patch we do an install, then do a rebuild hitting all the
major install code paths.

Fixes sass#1453
xzyfer added a commit to xzyfer/node-sass that referenced this issue Nov 7, 2019
This flag means we skip a bunch of code paths in CI which is not
ideal. Since we tell people to use `npm rebuild` we should eat our
own dog food.

With this patch we do an install, then do a rebuild hitting all the
major install code paths.

Fixes sass#1453
xzyfer added a commit to xzyfer/node-sass that referenced this issue Nov 7, 2019
This flag means we skip a bunch of code paths in CI which is not
ideal. Since we tell people to use `npm rebuild` we should eat our
own dog food.

With this patch we do an install, then do a rebuild hitting all the
major install code paths.

Fixes sass#1453
xzyfer added a commit to xzyfer/node-sass that referenced this issue Nov 7, 2019
This flag means we skip a bunch of code paths in CI which is not
ideal. Since we tell people to use `npm rebuild` we should eat our
own dog food.

With this patch we do an install, then do a rebuild hitting all the
major install code paths.

Fixes sass#1453
xzyfer added a commit that referenced this issue Nov 8, 2019
This flag means we skip a bunch of code paths in CI which is not
ideal. Since we tell people to use `npm rebuild` we should eat our
own dog food.

With this patch we do an install, then do a rebuild hitting all the
major install code paths.

Fixes #1453
@xzyfer

This comment has been minimized.

Copy link
Contributor

@xzyfer xzyfer commented Nov 8, 2019

Closed by #1734

@xzyfer xzyfer closed this Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.