-
Notifications
You must be signed in to change notification settings - Fork 504
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
Don't include map files in distributions #285
Don't include map files in distributions #285
Conversation
Wow I did not know map files were so big! |
Source maps tend to be on the big side. There is one argument for distributing them normally, which would to get clearer errors copy-pasted by users from their browser console, but I'm not sure how much that is worth :) |
Rather than deleting them, should we not produce them at all in "production
mode"?
(For publishing the npm package and the pypi package).
S.
…On Fri, Jul 5, 2019, 17:46 Vidar Tonaas Fauske ***@***.***> wrote:
Source maps tend to be on the big side. There is one argument for
distributing them normally, which would to get clearer errors copy-pasted
by users from their browser console, but I'm not sure how much that is
worth :)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#285>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AASJOFTS3APQALFQLO3TIEDP55UGXANCNFSM4H5F6TOQ>
.
|
An alternative approach would also be to distribute them in a separate package: conda-forge/mlflow-feedstock#9 |
You may still accidentally release them if they are generated (and didn't clean up). I think we still want to have source maps at this moment to have reasonble error messages. |
This is generally useful, but since the voila frontend is rather minimal, it might not be worth shipping them? |
@jtpio @maartenbreddels I am thinking of getting this in for 0.2.0. I like the reduction of the bundle size. |
Just to make sure: You are talking about the sdist size right? |
sdist and wheel (and thus conda package) I guess, what else could it be? |
EID:T I guess I wasn't used to thinking of sdist/wheels as "bundles", but instead I got concerned that the maps had been included in the JS bundle, instead of served separately ? |
I'd say we merge this, agree @SylvainCorlay?. I prefer this solution to not generating the map files since it might be useful for local debugging, and if it gets accidentally gets generated, we don't accidentally distribute it. |
Do we want to distribute this in |
Let do a release with this first, it's easy to revert or change. |
This reduces the size of the
sdist
from4.5M
to1.3M
for me.These files are quite useful during development but not needed by endusers.