Skip to content
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

Drop generated versions from the repo? #239

Open
tidoust opened this issue Sep 26, 2019 · 10 comments
Open

Drop generated versions from the repo? #239

tidoust opened this issue Sep 26, 2019 · 10 comments
Milestone

Comments

@tidoust
Copy link
Member

tidoust commented Sep 26, 2019

@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
Copy link
Member

wolenetz commented Nov 8, 2019

Hmm. Up until the most recent commit to media-source.html (8b236ed), 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
Copy link
Member

wolenetz commented Nov 8, 2019

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
Copy link
Member Author

tidoust commented Nov 19, 2019

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
Copy link
Member

@wolenetz , this might have been an oversight indeed.

@wolenetz
Copy link
Member

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.

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
Copy link
Member Author

tidoust commented Feb 3, 2020

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.

@mwatson2 mwatson2 added the agenda Topic should be discussed in a group call label Sep 21, 2020
@wolenetz wolenetz added this to the V2BugFixes milestone Sep 28, 2020
@mwatson2 mwatson2 removed the agenda Topic should be discussed in a group call label Sep 28, 2020
@wolenetz
Copy link
Member

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

tidoust added a commit to tidoust/media-source that referenced this issue Sep 29, 2020
The `media-source.js` script is used to manage cross-references in MSE specs.
To be able to reference this script from dedicated repositories for byte stream
formats and registry specifications (see w3c#239), the script must use an absolute
URL for the ED of the main Media Source Extensions spec.

The update also fixes a few broken fragments (naming rules have changed in
Respec in the meantime).
@tidoust
Copy link
Member Author

tidoust commented Sep 29, 2020

@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 added a commit that referenced this issue Nov 23, 2020
The `media-source.js` script is used to manage cross-references in MSE specs.
To be able to reference this script from dedicated repositories for byte stream
formats and registry specifications (see #239), the script must use an absolute
URL for the ED of the main Media Source Extensions spec.

The update also fixes a few broken fragments (naming rules have changed in
Respec in the meantime).
tidoust added a commit to tidoust/media-source that referenced this issue Nov 23, 2020
The registry and byte stream format specifications now each have their own
repository. This update replaces the specifications with redirects to these
repositories and notes the change in the main README.

See w3c#239 (comment)
@tidoust
Copy link
Member Author

tidoust commented Nov 23, 2020

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
Copy link
Member

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

tidoust added a commit that referenced this issue Nov 24, 2020
The registry and byte stream format specifications now each have their own
repository. This update replaces the specifications with redirects to these
repositories and notes the change in the main README.

See #239 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants