-
Notifications
You must be signed in to change notification settings - Fork 2
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
130 unpack main #29
Comments
Comment by Ebonsignori Amazing. How was this accomplished? Did you write Edit: Checked out and everything seemed to be working smoothly. |
Comment by activescott @Ebonsignori I wrote If you can test this branch a bit and confirm things work that would be much appreciated! |
Comment by JonathonDunford So, the only thing that needs to be done is to run the script and change that one require line? My initial concern is we wouldn't be able to see all changes made, but we could verify that the script to unpack works as intended and then run it ourselves. I tested it and it works. Awesome work. |
Comment by activescott @JonathonDunford Almost. There is one step to manually extract the array of modules from As long as a couple of us do a solid test pass and it works I think we should merge it. I suggest we get any other PRs that impact the main-unminified.js in first (only #125 AFAIK), then re-run the scripts to get the latest changes. We do a final quick test pass and then merge it. Once we do the merge though all bugs/enhancements would of course be in the new unpacked code 🎉 . I don't really expect any problems (Famous last words I know). I expect either everything will work or almost nothing would or there would be something glaringly wrong. If something were wrong, my guess is a whole module got missed, and it would be easy to track it down and unpack it manually in the worst case. |
Comment by activescott @forkdelta/frontend-team Would be great if you can all download this branch and run through some tests. Would really love some testing on the most critical operations like buy, sell, transfer, withdrawal, deposit, etc. On a related note, do we have a list of test cases anywhere? |
BTW: Let me send a new PR for this that I'm working on in branch at https://github.com/activescott/FrontEnd/tree/unpack_main |
@activescott What's the status on this? I don't see a PR in frontend |
PR is at #35 |
Issue by activescott
Sunday Mar 18, 2018 at 10:58 GMT
Originally opened as https://github.com/forkdelta/forkdelta.github.io/pull/131
NOTE: I suggest we ignore the content below here and use the PR for the latest information at #35
This definitely needs a lot of testing but appears to be working.
To test just checkout the branch and run as usual.
To re-build
js/main-bundle.js
from the modules runnpm run build
from the root directory.To unpack the modules from the old minified code again
js/main-unminified.js
and put it inscripts/_old_main_packed_modules.js
note: If you look carefully this whole ~150K line array is an argument to a function, so I can't think of a clean way to grab it from the script without more sophisticated (and error pone) parsing. Alternatively, if this branch is going to live for more than a week or so we could just adjustjs/main-unminified.js
to pop that array out into a field in the file and then I could safely get at it from the script. Since this is a one-time thing though I think it is just as easy to just manually extract it.scripts
, runnode unpack_modules.js
(it unpacks fromscripts/_old_main_packed_modules.js
)scripts/unpacked_modules/main.js
there is onerequire('../server/utility.js')
that needs converted torequire('./server/utility.js')
cp -r scripts/unpacked_modules/* js/
js/main-bundle.js
by runningnpm run build
from the root directoryThis is a one-time thing so we shouldn't need the script or to re-run this unless we need it for some merges before this gets merged.
activescott included the following code: https://github.com/forkdelta/forkdelta.github.io/pull/131/commits
The text was updated successfully, but these errors were encountered: