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

Editorial: Align with Web IDL specification #491

Closed
wants to merge 0 commits into from

Conversation

autokagami
Copy link
Contributor

@autokagami autokagami commented Oct 18, 2020

This is an automated pull request to align the spec with the latest Web IDL specification.

Currently the autofix might introduce some awkward code formatting, so please feel free to modify the formatting.

Please file an issue on https://github.com/saschanaz/webidl-updater/issues/new if you think this PR is invalid or should be enhanced.

The following is the validation messages from webidl2.js, which may help understanding this PR:

Validation error at line 8 in webvtt1,0, inside `interface VTTCue`:
 Constructor(double startTime,
 ^ Constructors should now be represented as a `constructor()` operation on the interface instead of `[Constructor]` extended attribute. Refer to the [WebIDL spec section on constructor operations](https://heycam.github.io/webidl/#idl-constructors) for more information.

Validation error at line 3 in webvtt1,1, inside `interface VTTRegion`:
 Constructor]
 ^ Constructors should now be represented as a `constructor()` operation on the interface instead of `[Constructor]` extended attribute. Refer to the [WebIDL spec section on constructor operations](https://heycam.github.io/webidl/#idl-constructors) for more information.

πŸ’₯ Error: 400 Bad Request πŸ’₯

PR Preview failed to build. (Last tried on Nov 2, 2020, 8:20 AM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 CSS Spec Preprocessor - CSS Spec Preprocessor is the web service used to build Bikeshed specs.

πŸ”— Related URL

Error running preprocessor, returned code: 2.
WARNING: The following <var>s were only used once in the document:
  'stylesheets', in algorithm 'WebVTT parser algorithm'
  'input', in algorithm 'WebVTT region settings parsing'
  'input', in algorithm 'WebVTT region objects'
  'regions', in algorithm 'collect WebVTT cue timings and settings'
  'input', in algorithm 'parse the WebVTT cue settings'
  'regions', in algorithm 'parse the WebVTT cue settings'
  'language', in algorithm 'WebVTT cue text parsing rules'
  'cue', in algorithm 'WebVTT rules for extracting the chapter
title'
  'cue', in algorithm 'WebVTT rules for extracting the chapter title'
  'language', in algorithm 'Processing model'
  'language', in algorithm 'rules for updating the display of WebVTT text tracks'
  'nodes', in algorithm 'Processing cue settings'
  'left', in algorithm 'apply WebVTT cue settings'
  'top', in algorithm 'apply WebVTT cue settings'
  'max dimension', in algorithm 'apply WebVTT cue settings'
  'video', in algorithm 'Obtaining CSS boxes'
  'startTime', in algorithm 'VTTCue construction'
  'endTime', in algorithm 'VTTCue construction'
  'text', in algorithm 'VTTCue construction'
If these are not typos, please add an ignore='' attribute to the <var>.
FATAL ERROR: The [Constructor] extended attribute (on VTTCue) is deprecated, please switch to a constructor() method.
 ✘  Did not generate, due to fatal errors

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

Copy link
Member

@foolip foolip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has resulted in the correct changes. Thanks @saschanaz!

It will require updating index.html, however.

@foolip
Copy link
Member

foolip commented Oct 23, 2020

@gkatsev trying to sort this out, I'm not sure that I understand the current repo setup. Both the master and gh-pages branch exists, but gh-pages is the default branch. index.html needs to be updated manually.

Here's an idea to simplify for contributors:

  • fork a new main branch from the current gh-pages and make that the default
  • configure https://github.com/w3c/spec-prod to automatically build and publish the index.html
  • delete the old master branch to avoid confusion about which branch is the active one

WDYT?

(I've come poking around because of whatwg/html#5297 (comment).)

@gkatsev
Copy link
Collaborator

gkatsev commented Oct 28, 2020

@foolip thanks for your patience. gh-pages is the branch that we've been using for a long time. I've personally never used master. I think gh-pages was the default from back when you couldn't have gh-pages from non-ghpages branch.

Making a new main from gh-pages, deleting master, and setting GitHub to serve from main sounds good to me.

I have a bunch of PRs against gh-pages branch but I can update them against the new main.

@foolip
Copy link
Member

foolip commented Oct 29, 2020

@gkatsev sounds great! Would you mind create a main branch at the same commit that gh-pages is currently at? I could then create a PR against that branch to publish to gh-pages using GitHub Actions. With that in place, it would be possible to merge this PR without having to manually update index.html afterwards.

@foolip
Copy link
Member

foolip commented Oct 29, 2020

(One would also have to change the default branch to main, and that would automatically change the target of all open PRs.)

@gkatsev
Copy link
Collaborator

gkatsev commented Oct 29, 2020

Ok, created main branch. Made it default and have gh-pages publishing from it as well. PRs may need to be manually updated. Couldn't find an option to automatically update the existing PRs.

@autokagami autokagami changed the base branch from gh-pages to main November 2, 2020 08:20
@autokagami autokagami closed this Nov 2, 2020
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

Successfully merging this pull request may close these issues.

None yet

3 participants