search

w3c / media-source

Media Source Extensions
https://w3c.github.io/media-source/
Other
267 stars 57 forks source link

Drop generated versions from the repo? #239

Open tidoust opened 4 years ago

tidoust commented 4 years ago

@wolenetz, @mwatson2,

There was some discussion during TPAC on switching MSE (and EME) to the latest version of ReSpec. There is no a priori problem doing so, although some editorial updates are likely going to be necessary. Happy to help if needed.

On top of that, I note that the structure of the repository is a bit unusual. Common practice with Respec documents is to only commit the source spec. This repository contains both the source (files ending with -respec) and the generated version of the spec. There may be good reasons why things got done that way. I personally find it a bit clumsy since it means both representations need to be kept in sync manually.

Do you want things to stay that way? If not, I suggest to replace the generated versions with the source files, and to get rid of the -respec files. Happy to prepare a PR for that, but I wanted to get your perspective first.

Also, what is the difference between index.html and media-source.html?

wolenetz commented 4 years ago

Hmm. Up until the most recent commit to media-source.html (8b236edf0be9f8dc2c0f2d6a14b6c37332d479a3), it had simply redirected to index.html. @plehegar, was this an oversight? (We historically kept the draft in index.html, such that it would be visible directly via https://w3c.github.io/media-source/.)

@acolwell et al setup the original structure, and I presume there was some guidance around that at the time which may differ now. I found the generated versions useful since they were snapshots that could be seen better in historical context (for instance, as respec versions/external links/etc changed, generated versions' history would show what the full context for that version of the spec was).

Keeping the generated versions updated required some editorial overhead, but not extreme IMHO.

wolenetz commented 4 years ago

Regarding updating to the latest version of respec, I agree it'll need to be done. This is partly why the generated spec snapshots can be helpful (they highlight respec and other issues earlier). Another example of how generated spec snapshots are helpful is they can be htmldiff'ed to highlight pertinent portions of the generated result that change across the diff.

I'm certainly open to suggestions or new workflows though. Please keep these questions and suggestions/recommendations coming!

tidoust commented 4 years ago

I confess I don't really understand why generated snapshots can help highlight respec and other issues earlier. Having things dynamic means that new ReSpec warnings/errors may pop up at any time even when the raw source does not change. But then that also means that these warnings/errors are reported as soon as possible, instead of only when merging a PR. That's all up to you in the end, I don't mean to impose anything.

Regarding htmldiff-ing, what could actually be useful is to setup PR Preview to auto-link pull requests to a human-readable preview and diff. PR Preview works directly with ReSpec source files (it generates the result on-the-fly through an instance of spec-generator running on W3C servers). PR Preview is e.g. running on the Media Capabilities repo (that spec uses Bikeshed, but the principle is the same).

plehegar commented 4 years ago

@wolenetz , this might have been an oversight indeed.

wolenetz commented 4 years ago

w.r.t. PR-Preview, I have a PR out right now (https://github.com/w3c/media-source/pull/252) to enable that for the main MSE spec respec file (media-source-respec.html). Unfortunately, PR-Preview only supports one respec file per repo (https://github.com/tobie/pr-preview/issues/18), so the various bytestream formats and their registry respecs do not benefit from pr-preview. However, they're much smaller and changes to them are already much easier to review as a result.

Regarding the generated-and-checked-in-snapshots allowing for clearer identification of respec and other issues, my apologies for confusing the issue. The very act of putting forth a generated file for review includes the various respec warnings/etc in the actual commit. This way, the reviewer also can see the various problems. PR-Preview might afford similar. I think the main supporting argument for keeping checked-in generated snapshots in the repo is to observe them in their context (e.g., external references, etc at the time the snapshot was generated versus now). For extension specs like MSE, where the spec being extended (HTML5) might get out of sync over time, having such context explicit in the snapshot can help resolve the inconsistencies.

tidoust commented 4 years ago

w.r.t. PR-Preview, I have a PR out right now (#252) to enable that for the main MSE spec respec file (media-source-respec.html). Unfortunately, PR-Preview only supports one respec file per repo (tobie/pr-preview#18), so the various bytestream formats and their registry respecs do not benefit from pr-preview. However, they're much smaller and changes to them are already much easier to review as a result.

Given the relationship between the specs, you may prefer to keep them in the same repo and that's totally fine, but I note we typically encourage groups to adopt a one repo per spec rule, precisely because it makes integration with tooling easier. If that seems useful, I'm happy to take care of repos creation and setup.

wolenetz commented 3 years ago

@tidoust, let's do as you suggest. Please setup repos specifically for each spec.

tidoust commented 3 years ago

@tidoust, let's do as you suggest. Please setup repos specifically for each spec.

I created one for the registry spec:

Please note that the links to the main MSE spec from that spec are currently broken because the media-source.js needs to be updated (see PR #258).

Also note that I dropped the notion of source / generated file.

Does the new repo look good enough? If so, I'll do the same for the other specs.

tidoust commented 3 years ago

I created one for the registry spec:

I have now migrated all byte stream format specs to their own repositories:

Apart from a couple of fixes to update the specs to the latest version of ReSpec, the content of these specifications has not changed.

Please note that the links to the main MSE spec from that spec are currently broken because the media-source.js needs to be updated (see PR #258).

I merged that one. If there are remaining broken links, that's a mistake!

PR #260 makes the final update to have all links point to the new repositories.

wolenetz commented 3 years ago

All these generally look good to me. Thank you for doing this work.