-
Notifications
You must be signed in to change notification settings - Fork 75
Use rollup #94
base: master
Are you sure you want to change the base?
Use rollup #94
Conversation
| app.set('views', __dirname + '/views'); | ||
| app.set('view engine', 'ejs'); | ||
| app.use(express.static('public')); | ||
| app.use(express.static(__dirname + '/public')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any idea where it pulls public from without setting _dirname + '/... ?
I just opened #96 which would conflict with this change, but without making the change mbview was broken for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My guess would express.static looks for "public" in your current working directory.
I couldn't find any docs with a definite answer.
None of
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find a single directory with that name, even in node_modules. You're definitely safe to nuke that line.
# in root of repo
docker run -it $(pwd):/root node:12-alpine sh
cd && rm -rf ./node_modules && npm ci && find . -type d -name public
echo $? && exit
Addresses #56, #42, and #84.
Tests should all pass, but before approving, you should verify the raster layer works as expected. https://github.com/klokantech/vector-tiles-sample has an example raster .mbtiles that you can download, though it's only available at low zoom. I didn't write tests on top of that since the repo is unlicensed (though I did put in a ticket about that).