Vendor/copy in remaining jquery libraries to assets?
StephanDollberg opened this issue · 5 comments
Hi,
Would anything speak against copying in the remaining JS/CSS libaries that are currently loaded from CDN to the assets folder as well?
It would make using optview2 on private networks a lot easier as one wouldn't have to download assets manually and hack the python script.
All jquery stuff is MIT licensed so I don't think that would be a massive problem and one of the deps is already copied in.
Cheers,
Stephan
PS.: Great talk at CppCon!
@StephanDollberg Thanks!
Copying the dependencies into ./assets means optview2 will have them frozen in time, and not receive future improvements or bug fixes (security fixes too).
I understand limited networks pose an obstacle - I can suggest building the included test project running opt-viewer on it (might be as simple as ./cpp_optimization_example/run_optview2.sh) on some non-limited network, and then copying the optview2 folder (with dependencies now downloaded) into the dest machine. Would that be good enough?
Copying the dependencies into ./assets means optview2 will have them frozen in time, and not receive future improvements or bug fixes (security fixes too).
The versions are currently hardcoded at https://github.com/OfekShilon/optview2/blob/main/opt-viewer.py#L155-L157 so I don't see how that would make a difference? I do understand the point though but not sure how important that is for something like optview.
I can suggest building the included test project running opt-viewer on it ...
I don't fully follow here, the js/css is being loaded from the browser so wouldn't end up in assets even after running once?
@StephanDollberg Sorry, my bad - I now understand which dependencies you're referring to. I pushed a fix per your suggestion, would appreciate if you could give it a spin and say if it works for you.
I think the index page here https://github.com/OfekShilon/optview2/blob/main/opt-viewer.py#L248-L250 also needs adapting other than that should be good.
@StephanDollberg Pushed a fix, thanks again.