OpenLightingProject / open-fixture-library

A library and website for lighting technology's DMX fixture definition files.
https://open-fixture-library.org/
MIT License
188 stars 63 forks source link

More URLs in the schema #434

Closed FloEdelmann closed 6 years ago

FloEdelmann commented 6 years ago
  1. to an official manufacturer product page
  2. to the fixture page in OFL

EDIT: see my comment below

FloEdelmann commented 6 years ago

Maybe instead of 2., we should rather add manufacturer and fixture key, so that the fixture page URL (and maybe other things) can be constructed from those.

ameisso commented 6 years ago

Hello @FloEdelmann,

Deducting the URL from parameters seems a good idea. This will allow you to change the app url without modifying all the fixture files.

Best

ameisso commented 6 years ago

Hello @FloEdelmann,

Please see here an attempt to have a direct link to your site in the fixture definition.

If thats ok for you, i'll send a pull request for that.

Best

FloEdelmann commented 6 years ago

Maybe a Millumin export plugin is the best idea for the moment; but I'd rather suggest updating our own format since having to maintain two export plugins that output essentially our format seems a bit overkill to me. I just noticed that the OFL output plugin's readme is also a bit outdated and could link to the docs to clarify things.

If you like, you can add the OFL fixture page link to the OFL export plugin and mention Millumin in its readme.

On the other hand, I also see an advantage of your suggested solution: users will instantly know which format to download a fixture in (Millumin is explicitly mentioned in the dropdown). Will you bundle fixtures with the program (and update and extend them regularly with newer versions) or require users to download them theirselves?

ameisso commented 6 years ago

Hello @FloEdelmann,

I've just pushed the modifications to OFL JSON scheme. After reflection, it could be a good idea to keep the 2 formats (OFL and Millumin) thus allowing you to make versions changes and leave us the time to adapt or code.

In the end, we will leave the user download the fixtures by himself from your site. So keeping the Millumin input will simplify this operation.

I've also changed the wording 'oflLink' to 'oflURL' Best

FloEdelmann commented 6 years ago

I think it'd be best to open a WIP (work in progress) pull request, so we can track code changes after discussion more easily.

Keeping the current JSON format schema as a separate Millumin plugin seems like a good idea – unfortunately also like more effort on our side :sweat_smile:

fxedel commented 6 years ago

[...] keep the 2 formats (OFL and Millumin) thus allowing you to make versions changes and leave us the time to adapt o[u]r code.

@ameisso Unfortunately, this isn't that easy. Both the OFL and Millumin plugins take the current up-to-date fixture JSON and only add/change some top-level properties ($schema, oflURL, etc). So when we introduce a new feature, the Millumin-exported fixtures also already include it.

There are some backwards-compatible changes (like adding the new channel type: "Color Temperature") that could be implemented by Millumin before we actually merge it at OFL, which allows for a seamless upgrade.

However, this would require us to wait for other consumers of our format (currently only Millumin) before we can change our schema. It also doesn't work with incompatible schema changes.

Another possibility is to add some logic to the Millumin plugin that "downgrades" new features to the latest supported schema version. If we implemented that logic for each atomic version change, we could reuse it amongst multiple plugins that support different schema versions in future.

Keeping the current JSON format schema as a separate Millumin plugin seems like a good idea – unfortunately also like more effort on our side :sweat_smile:

@FloEdelmann Ah, you commented whilst I was still composing this comment :smile:. I guess you refer to the downgrading solution with "more effort"?

FloEdelmann commented 6 years ago

@fxedel Correct.

ameisso commented 6 years ago

Hello @FloEdelmann and @fxedel,

Ok I got your point. and the PR is created

So if I summarise, maybe the best idea would be to recreate the same JSON as it is now and avoiding the upcomming change to perturbate our model ?

Is there a possibility in your schema to get the jsonObject from a specific schema or is this something we need to create ?

Best

fxedel commented 6 years ago

@ameisso

So if I summarise, maybe the best idea would be to recreate the same JSON as it is now and avoiding the upcomming change to perturbate our model ?

That'll be a possibility as well: Don't use the JSON directly, but generate it from the model. It's some work now, but less work in future.

My first proposal was to leave it as it is and with each new version, manipulate the JSON in the Millumin plugin so that it fits to the old version. What do you think?

Is there a possibility in your schema to get the jsonObject from a specific schema or is this something we need to create ?

Unfortunately not, the schema relies on the current JSON and doesn't create a JSON based on a schema. You could add this feature, though :)

FloEdelmann commented 6 years ago

I think this is not something we should focus on right now. In future schema versions, you will still be able to use some parts of the fixture JSON as is, while others will need to be recreated. We can't really distinguish between those parts now, so I think it'll be better to just update the plugin with each schema version change to provide backwards compatibility.

ameisso commented 6 years ago

Ok, so we leave this as is and will adapt it in the future. Thanks for the tips anyway.

When do you think that this could be merged into your repo ? (so we can publish our beta version).

Best

FloEdelmann commented 6 years ago

@ameisso see my comments in #466. As soon as the requested changes are resolved (and maybe some more README changes from our side), the PR can be merged.

FloEdelmann commented 6 years ago

More link types we should allow: Multiple manual URLs (like needed in #462), YouTube videos (which could be automatically embedded into fixture pages) and custom links.

Thus, it might make sense to change the way links are specified:

{
  "$schema": "https://raw.githubusercontent.com/OpenLightingProject/open-fixture-library/master/schemas/fixture.json",
  ...
  "meta": { ... },
  "URLs": {
    "manual": [
      "https://manufacturer.com/products/fixture/manual.pdf",
      "https://manufacturer.com/products/fixture/manual-old.pdf"
    ],
    "manufacturerDetails": ["https://manufacturer.com/products/fixture"],
    "youtube": [
      "https://youtube.com/watch?v=...",
      "https://youtube.com/watch?v=..."
    ],
    "custom": ["...", "..."]
  },
  "physical": {
    ...

The OFL and Millumin plugins would also add oflURL (as ofl) to this object.

ameisso commented 6 years ago

Hello,

Thanks for the info. Not sure that every fixture would have a youtube video. So I don't think embedding the youtube video is a good idea. Maybe under a link or something ?

Best

FloEdelmann commented 6 years ago

@ameisso Of course, all URLs would be optional. But if there is a YouTube video for a fixture (and a user enters its URL), it would be a nice addition.

We will let you know once we start working on this change (it might still take some time).

FloEdelmann commented 6 years ago

Embedding videos into the UI should happen with users' privacy in mind, so we should use embetty by heise online. This is not yet available as a Vue component, so we will have to rewrite it ourselves...

fxedel commented 6 years ago

When creating a new Vue component for embetty, we should definitely consider publishing it on npm!