Skip to content
This repository has been archived by the owner on May 30, 2019. It is now read-only.

Adds ./tools/dev_website for fast incremental builds #346

Merged
merged 1 commit into from Mar 10, 2018
Merged

Conversation

ry
Copy link
Contributor

@ry ry commented Mar 9, 2018

The deps commit includes some hacks on parcel to fix a bug - I will send upstream soon

propelml/propel_deps@4103536#diff-5e9e9af54dc82093dc2e5334dfff25c3

@piscisaureus
Copy link
Member

× C:\Users\BertBelder\d\propel3\website\main.scss:undefined:undefined: Missing binding C:\Users\BertBelder\d\propel3\deps\node_modules\node-sass\vendor\win32-x64-57\binding.node
Node Sass could not find a binding for your current environment: Windows 64-bit with Node.js 8.x

Found bindings for the following environments:

  • OS X 64-bit with Node.js 8.x

@piscisaureus
Copy link
Member

Nice. Although I am a bit concerned by the code duplication between build_website and dev_website.
Also note that the next time someone runs npm update your parcel hacks will be overwritten - this is going to be a pain unless you manage to upstream the pacel patch soon.

@ry
Copy link
Contributor Author

ry commented Mar 9, 2018

Patch sent upstream: parcel-bundler/parcel#974

@ry ry force-pushed the dev_website branch 3 times, most recently from ccc841a to b57e1b8 Compare March 10, 2018 17:51
Copy link
Member

@piscisaureus piscisaureus left a comment

Choose a reason for hiding this comment

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

LGTM, 1 comment

tools/build.js Outdated
@@ -6,6 +6,7 @@ if (process.argv.indexOf("clean") >= 0) {
}

(async() => {
run.sh("npm rebuild");
Copy link
Member

Choose a reason for hiding this comment

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

This fails on windows. Cannot spawn .cmd files. Use execSync instead.

Copy link
Member

Choose a reason for hiding this comment

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

Actually can we not have this here at all? npm rebuild is way too slow for me to want to do that all the time.

tools/build.js Outdated
@@ -6,6 +6,7 @@ if (process.argv.indexOf("clean") >= 0) {
}

(async() => {
run.sh("npm rebuild");
Copy link
Member

Choose a reason for hiding this comment

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

Actually can we not have this here at all? npm rebuild is way too slow for me to want to do that all the time.

@ry
Copy link
Contributor Author

ry commented Mar 10, 2018

@piscisaureus Removed node-sass and used a symlink to sass. Removed the npm rebuild too. PTAL.

For fast incremental website builds
@ry ry merged commit b33a014 into master Mar 10, 2018
@ry ry deleted the dev_website branch March 10, 2018 19:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants