-
-
Notifications
You must be signed in to change notification settings - Fork 77
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
Created new output format HighCharts #18
Conversation
Is it acceptable to add dependencies to higcharts in the form of a submodule? or is it preferred if i hardcopy it? |
currently a single chart: FrequencyHistogram This format is intended to be a simple way of adding specific use case charts Added temporary solution for: #15
Some general comments (I haven't looked at the implementation):
When using submodules it is not possible to control its dependencies because whatever version is released to the module (at the time of replication) is used for the installation which would allow for a version to be installed it was not tested against. While I would say that LicenseThe bigger problem I see is with the license (the reason I never touched highcharts) because as the website states:
Enabling it by default can potentially cause trouble for entities that not fulfill the above condition and therefore it must be clear to an end-user that additional obligations are attached to this format. [0] http://blog.appfog.com/getting-lean-and-solving-problems-farewell-to-git-submodules/ |
@JeroenDeDauw @kghbln @yaronkoren Do you have any comments on the mentioned license issue above? |
I think that the licence is indeed a big problem since it cannot be guaranteed that SRF is only used in non-commercial environments. In fact "Wiki of the Month" already proves that this is not the case. A way out will perhaps be to move this to an independent extension. Even then I strongly recommend to contact them for permission and to ask if this is an acceptable way. I still doubt they agree to something like this but it is probably still worth a try since these charts are really cool. Bundling this with SRF and not enable it by default does not work in my opinion since it is even harder to prove that one does not use it. |
Ill see what i can do about moving this into composer dependency How about at first run of any highcharts format, it would output the
|
Agree with @mwjames and @kghbln the license is problematic. An separate extension is indeed likely the best approach.
MediaWiki has not nice way of supporting something like this. And if you actually want to do it from on the wiki itself, then I'm not sure how you can do it without introducing a security liability. If it goes into SRF, there should be a clear warning for users in the docs. That warning would also be there for people that don't care about this format at all, making things more confusing for them. This makes me think it is simply easier to have it in its own extension. |
@mwjames Do you have any idea of how to add create a resource/asset in composer? i've created a library package of the highcharts javascript files, however they gets installed under /mediawiki/vendor instead of /mediawiki/extensions/ext/vendor. |
I've added this as its own extension. Extension:SemanticHighcharts |
Sorry I missed this conversation before - but I think a separate extension makes sense, yes. I should note that this is the first MediaWiki extension I know of that includes non-"free" code. Not that that's necessarily a problem; it's just interesting. |
currently a single chart: FrequencyHistogram
This format is intended to be a simple way of adding very specific charts
Added temporary solution for:
#15