Remove Makefile and migrate to NPM scripts #1135
Conversation
| @@ -39,6 +39,7 @@ | |||
| "jspngopt": "^0.2.0", | |||
| "less": "~2.7.1", | |||
| "less-loader": "^4.0.5", | |||
| "mkdirp": "^0.5.1", | |||
There was a problem hiding this comment.
For cross-platform mkdir
| @@ -104,7 +104,7 @@ git checkout --detach | |||
|
|
|||
| # Build generated files and add them to the repository (for bower) | |||
| git clean -fdx build dist | |||
| make setup dist | |||
| npm run dist | |||
There was a problem hiding this comment.
Should this command run when --dry-run?
There was a problem hiding this comment.
I think so. It won't do anything other than echo the command to the shell which is what we want in the dry run.
There was a problem hiding this comment.
@kevinbarabash Do you mean it should actually build the KaTeX or not?
There was a problem hiding this comment.
Sorry, the real command should not run, it should jus echo what the command to the console.
| git clone https://github.com/Khan/KaTeX-test-fonts test/screenshotter/unicode-fonts | ||
| cd test/screenshotter/unicode-fonts && \ | ||
| git checkout 99fa66a2da643218754c8236b9f9151cac71ba7c && \ | ||
| cd ../../../ |
There was a problem hiding this comment.
This is included in the dockers/Screenshotter/screenshotter.sh.
| TOTAL=`echo $${JSSIZE}+$${CSSSIZE} | bc`; \ | ||
| printf "Minified, gzipped js: %6d\n" "$${JSSIZE}"; \ | ||
| printf "Minified, gzipped css: %6d\n" "$${CSSSIZE}"; \ | ||
| printf "Total: %6d\n" "$${TOTAL}" |
There was a problem hiding this comment.
I'm not sure how to do this easily in the NPM scripts.
| PYTHON=$(shell python2 --version >/dev/null 2>&1 && echo python2 || echo python) | ||
|
|
||
| metrics: | ||
| cd metrics && $(PERL) ./mapping.pl | $(PYTHON) ./extract_tfms.py | $(PYTHON) ./extract_ttfs.py | $(PYTHON) ./format_json.py --width > ../src/fontMetricsData.js |
package.json
Outdated
| "test": "npm run prestart && npm run lint && npm run flow && npm run jest", | ||
| "screenshots": "npm run prestart && dockers/Screenshotter/screenshotter.sh", | ||
| "verify-screenshots": "npm run screenshots -- --verify", | ||
| "prestart": "node check-node-version.js && check-dependencies && cd src && node unicodeMake.js >unicodeSymbols.js", |
There was a problem hiding this comment.
As generating unicodeSymbols.js is cheap, it does before every tests and builds.
There was a problem hiding this comment.
One less thing to remember to do. :)
.arclint
Outdated
| @@ -2,7 +2,7 @@ | |||
| "linters": { | |||
| "katex-linter": { | |||
| "type": "script-and-regex", | |||
| "script-and-regex.script": "make lint || true", | |||
| "script-and-regex.script": "npm run lint || true", | |||
There was a problem hiding this comment.
Thanks for updating this file. We use phabricator/arcanist internally, but I can't remember the last time I saw a phabricator review for KaTeX.
| @@ -104,7 +104,7 @@ git checkout --detach | |||
|
|
|||
| # Build generated files and add them to the repository (for bower) | |||
| git clean -fdx build dist | |||
| make setup dist | |||
| npm run dist | |||
There was a problem hiding this comment.
I think so. It won't do anything other than echo the command to the shell which is what we want in the dry run.
| "dist": "npm test && npm run build && npm run dist:copy && npm run dist:zip && npm run dist:dist", | ||
| "dist:copy": "cd build && mkdirp katex && cp -r katex.js katex.min.js katex.css katex.min.css contrib fonts ../README.md katex", | ||
| "dist:zip": "cd build && tar czf katex.tar.gz katex/ && zip -rq katex.zip katex/", | ||
| "dist:dist": "rimraf dist/ && cp -r build/katex/ dist/", |
There was a problem hiding this comment.
I like this organization. Could you do something similar for test?
Codecov Report
@@ Coverage Diff @@
## master #1135 +/- ##
=======================================
Coverage 78.95% 78.95%
=======================================
Files 59 59
Lines 3872 3872
Branches 655 655
=======================================
Hits 3057 3057
Misses 674 674
Partials 141 141Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I'm okay with this as is. We can always split npm test into separate commands later. @ylemkimon I leave it up to you as to whether that change is part of the PR or not.
Fixes #1121.