Closed nosekefik closed 3 years ago
Sorry for the late answer, somehow I did not get a notification for the pull request.
It will take me a while to work through this and review it, because there are basically several features in this PR, but I'll get to it eventually. I usually prefer one feature per PR, but I guess that is (at least to some part) my fault because I haven't added any contribution guidelines - until today. :D But don't worry, you do not need to open another PR this time. I will handle that.
So at first glance I can say this:
publicURL
stuff should probably be a configuration option and not hardcoded.Update @nosekefik: The new release, version 1.1.0, does now contain the image size feature in a slightly reworked version.
Update 2 @nosekefik: The UUID part of commit c042daf has been included as new release 1.2.0: https://github.com/striezel/echarts-node-export-server/releases/tag/v1.2.0
Next Update @nosekefik: The new release 2.0.0 (https://github.com/striezel/echarts-node-export-server/releases/tag/v2.0.0) uses ECharts 5.0.2.
Although version 5.1.0 of ECharts has already been published to GitHub, the echarts.min.js file of that version does not seem to work with PhantomJS. Neither did version 5.0.0 of ECharts, but both version 5.0.1 and 5.0.2 seem to be just fine. Seems a bit like random hit and miss to me with ECharts >= 5.0.0.
Final update @nosekefik: I am closing this pull request now, because basically all features except for the publicURL
stuff of commit c042daf have been incorporated into version 2.0.0 or earlier version.
As mentioned earlier, the publicURL
changes should be configurable in some way or another. For example, it could be an additional command line option when starting the application or an environment variable.
Added option to set the image size, changed unix on filename by uuid, and updated echarts library