Closed tripu closed 5 years ago
The diff is a bit hard to review, so I'll focus on the intent rather than the details.
@dontcallmedom:
"uglifyjs is deprecated; use uglify-js instead; adapt scripts" SGTM ; I assume you tested the result, but let me know if you want me to take a stab at it as well
Yes, the change seemed OK in my (quick) tests.
"Remove node_modules/ (exclude from version control)" - wow, how did it end up there in the first place???
I know, right!? I didn't even know/remember that was so. It seems it was committed by accident — and then excluded… but not quite.
"Include js/midgard.min.js in version control" - I'm not a fan of including build files in source version control, but if that significantly simplifies deployment, I'll live with it
I don't feel strongly about this.
I saw we're including css/midgard.min.css
, and thought that was the policy here.
I'll revert that (ie, still exclude js/midgard.min.js
), and exclude also css/midgard.min.css
(to be consistent); and ping you to re-review.
@dontcallmedom: ready for your review again.
LGTM, thx
uglifyjs
is deprecated; useuglify-js
instead; adapt scriptsnode_modules/
(exclude from version control)js/midgard.min.js
in version controlThese are changes that I needed to do while deploying Midgard on a new server today.