-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fill in template contents for explainer repositories. #1
Conversation
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.
Two requested changes:
- The instructions say "look for TODO" but really, each section needs to be filled out - either a bunch of sections need TODO in them, or just say "fill all this out".
- Maybe I'm misunderstanding the Key Scenarios sections (or it needs retitling to "API interactions" or something), but it seems like it needs to come further up, where use cases and scenarios are listed.
|
||
## Proponents | ||
|
||
- [Proponent team 1] |
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.
Should these have "todo" on them, or should the instructions say to look for brackets?
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.
I hope moving the "TODO: Fill in the whole explainer template" bit above this is sufficient.
README.md
Outdated
- [etc.] | ||
|
||
## Participate | ||
- [Issue tracker] |
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.
As above: todo here?
README.md
Outdated
|
||
## Introduction | ||
|
||
TODO: Fill this in using https://tag.w3.org/explainers/ as a reference. |
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.
It's not clear if this TODO is for the entire following sections or not?
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.
I've moved it up to the top of the explainer and reworded a bit so it's clear it covers the whole thing.
README.md
Outdated
|
||
[If spec work is in progress, link to the PR or draft of the spec.] | ||
|
||
## [Potential Solution 2] |
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 is possible, but relatively unlikely (that a second+ solution would be documented, right? Perhaps we should remove this, but above say "there may be more than one potential solution section".
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.
Sounds good.
README.md
Outdated
<!-- In your initial explainer, you shouldn't be attached or appear attached to any of the potential | ||
solutions you describe below this. --> | ||
|
||
## [Potential Solution 1] |
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 potential solution(s) should be below the "tricky design choice" section below, but DEFINITELY after the Key Scenarios.
README.md
Outdated
and a brief overview of how the solution works. | ||
This should be no more than 1-2 paragraphs.] | ||
|
||
## Goals [or Motivating Use Cases, or Scenarios] |
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.
Not clear how this interacts with the Key Scenarios below? Definitely need to make sure htis gets filled out, in depth, prior to the potential solution.
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.
Yeah. I took this directly from the TAG's template, but it's not clear. I think the idea was to list use cases here, and then have the Key Scenarios explain how the potential solutions solve the use cases. I'll try to make that clearer ... and then also send the TAG a patch.
index.bs
Outdated
|
||
For now, see the [explainer]([REPOSITORYURL]). | ||
|
||
See [https://speced.github.io/bikeshed/](https://speced.github.io/bikeshed/) to get started on your specificaton. |
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.
Drop in links to the other "how to Bikeshed" and "how to spec" links? So people have several jumping off points, besides just the (sometimes intimidating) Bikeshed docs.
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.
Done.
index.bs
Outdated
Repository: explainers-by-googlers/todo-your-repo-name | ||
URL: https://explainers-by-googlers.github.io/todo-your-repo-name | ||
Editor: TODO: Your Name, Google https://google.com, [email protected] | ||
!Tests: <a href=https://github.com/w3c/web-platform-tests/tree/master/todo-api-label>web-platform-tests todo-api-label/</a> (<a href=https://github.com/w3c/web-platform-tests/labels/todo-api-label>ongoing work</a>) |
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.
Is there a particular reason you're suggesting an explicitly-constructed WPT line, rather than using the Bikeshed's WPT features directly? This method won't, for example, fill in any test results.
Suggest instead a WPT Path Prefix: TODO-API-LABEL
and WPT Display: closed
, along with a <wpt rest></wpt>
at the end of the body along with a short note explaining how to properly sprinkle <wpt>
thruout the document.
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 is what npx wicg
produced: https://github.com/WICG/starter-kit/blob/main/templates/index.bs. I've taken your suggestion, and I'll update that once this template is merged.
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.
Thanks for the reviews. How's this?
index.bs
Outdated
Repository: explainers-by-googlers/todo-your-repo-name | ||
URL: https://explainers-by-googlers.github.io/todo-your-repo-name | ||
Editor: TODO: Your Name, Google https://google.com, [email protected] | ||
!Tests: <a href=https://github.com/w3c/web-platform-tests/tree/master/todo-api-label>web-platform-tests todo-api-label/</a> (<a href=https://github.com/w3c/web-platform-tests/labels/todo-api-label>ongoing work</a>) |
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 is what npx wicg
produced: https://github.com/WICG/starter-kit/blob/main/templates/index.bs. I've taken your suggestion, and I'll update that once this template is merged.
index.bs
Outdated
|
||
For now, see the [explainer]([REPOSITORYURL]). | ||
|
||
See [https://speced.github.io/bikeshed/](https://speced.github.io/bikeshed/) to get started on your specificaton. |
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.
Done.
README.md
Outdated
|
||
[If spec work is in progress, link to the PR or draft of the spec.] | ||
|
||
## [Potential Solution 2] |
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.
Sounds good.
README.md
Outdated
|
||
## Introduction | ||
|
||
TODO: Fill this in using https://tag.w3.org/explainers/ as a reference. |
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.
I've moved it up to the top of the explainer and reworded a bit so it's clear it covers the whole thing.
|
||
## Proponents | ||
|
||
- [Proponent team 1] |
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.
I hope moving the "TODO: Fill in the whole explainer template" bit above this is sufficient.
README.md
Outdated
and a brief overview of how the solution works. | ||
This should be no more than 1-2 paragraphs.] | ||
|
||
## Goals [or Motivating Use Cases, or Scenarios] |
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.
Yeah. I took this directly from the TAG's template, but it's not clear. I think the idea was to list use cases here, and then have the Key Scenarios explain how the potential solutions solve the use cases. I'll try to make that clearer ... and then also send the TAG a patch.
index.bs
Outdated
and [https://speced.github.io/bikeshed/](https://speced.github.io/bikeshed/) to get started on your | ||
specificaton. | ||
|
||
<wpt rest></wpt> |
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.
Hmmm, @tabatkins this spelling produces an "Attribute “rest” not allowed on element “details” at this point." error in spec-prod's markup validation. Using <wpt-rest>
as suggested in https://speced.github.io/bikeshed/#elementdef-wpt-rest fails Bikeshed with
FATAL ERROR: Couldn't find any tests with the path prefix 'TODO-API-LABEL'.
I could just remove this or comment this out, since the issue tells people what to do once they set their WPT Path Prefix. What do you think?
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.
Sorry, yes, <wpt-rest>
is what I meant.
Re: the fatal error; go ahead and just ignore my suggestion for a wpt-rest
, actually. It works just fine as-is; it (currently) won't complain at all about a nonsense path prefix as long as you don't actually use any wpt-related elements. But when that TODO is replaced with the actual path prefix, it'll start complaining about tests not being specified, which is good. (Tho I really need to work on the error message...)
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.
Removed. :)
When the spec author fills in the WPT Path Prefix, it'll give them errors that say to add <wpt> elements, which is as good as an inline issue.
SHA: 2e383d8 Reason: push, by jyasskin Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@tabatkins, let me know what changes you suggest for the initial Bikeshed file.