-
Notifications
You must be signed in to change notification settings - Fork 64
Correctly format JSON in configuration file #375
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
base: develop
Are you sure you want to change the base?
Conversation
| "plugins": { | ||
| "enablePluginsByDefault": false, | ||
|
|
||
| "//**** Instructions: Disable any individual plugin by setting its enable property to false": {"enabled": false}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This really is obvious, isn't it?
| "enablePluginsByDefault": false, | ||
|
|
||
| "//**** Instructions: Disable any individual plugin by setting its enable property to false": {"enabled": false}, | ||
| "//**** For a list of available plugins and configuration, go to": "https://github.com/polimediaupv/paella/blob/master/doc/plugins.md", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The URL is broken anyway.
| "es.upv.paella.videoZoomTrack4kPlugin": { "enabled":true, "targetStreamIndex":0, "autoModeByDefault":false }, | ||
| "es.upv.paella.airPlayPlugin": { "enabled":true }, | ||
|
|
||
| "//***** Video Overlay Button Plugins": "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the value of such "comments" in a configuration file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@staubesv I find these short, releavant comments, that are close to the data, to be extremenly useful guides. The same way that short relevant comments in the Opencast properties file and code give a boost of context and design motivation. The alternative is to require users to parse through a local README or compile a remote set of markup docs. It's most likely, and reinforced by the "obvious" comment, that people struggle with what they consider to be obvious based on property names, and only read the instructions as a last resort. The inline comments gives greater accessibility to the obvious.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@karendolan Can you provide an example of a relevant comment? At least for me, it doesn't help me anything to know that a plugin that I can enable is of a specific type (e.g. Video Overlay Button Plugin). To know, for example, what enabling es.upv.paella.videoZoomTrack4kPlugin will cause I need a documentation of that plugin.
Since JSON does not support comments, it seems the wrong place to add such information as description etc.
Anyway... no strong opinion about the comments. In first place, this PR is about having a properly formatted JSON.
Note though that automatic formatters will not deliver good results for data that actually is supposed to be "comments" as this would require a different formatting for optimum results.
This addresses #374
The JSON configuration file of the Paella player has many incorrect formattings. The goal of this task (it is neither a feature nor a bug) is to use a JSON formatter to get consistently formatted json.
I will use https://jsonformatter.org for this.
Also, the configuration file is working around the fact that JSON does not support comments. Most of the comments seem useless (how to disable/enable plugins is really obvious) or out-dated (e.g. reference to plugins.md on github which is a dead link).
It would be preferable to have the documentation separated and the individual plugins to be documented.
Note that this PR does not change any of the configuration settings. It just does...