-
Notifications
You must be signed in to change notification settings - Fork 6
Add Bikeshed support #712
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
Add Bikeshed support #712
Conversation
This is what's used in other w3c repos I've encountered. Now is a good time to switch, since this PR is already heavily touching all of the TS code in the repo anyway.
- Adds md-<key> and die-on support to ReSpec - Expands tests and fixes test script to actually run all of them - Updates watch script to also restart on HTML change - TODO: Add die-on tests for ReSpec
535d0cf to
ba384e0
Compare
| type="radio" | ||
| name="type" | ||
| value="respec" | ||
| checked |
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.
Small enhancement: If url parameter has ?type set, we can check the right radio buttons with script.
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 use case for this enhancement? I ask because this would expose multiple edge cases if the user were to change the Document type choice after loading this way:
- Doing this client-side would override any previous user input within these radio buttons if the user were to refresh the page or revisit the form via browser history
- Doing this server-side would avoid this, but would involve adding more complexity and dependencies to the Express setup (which I had considered doing earlier in this PR, but later decided to avoid when merging back to a single page)
- GET parameters currently override POST parameters if both are present; I'd need to flip this, otherwise the form submission (which uses
action=""to work regardless of base path) will break if a user changes the Document type, then performs a file upload withtypein the URL- Changing this might be fine; many
curluse cases found across GitHub use GET parameters with a POST request, but are unlikely to define the same parameter both ways
- Changing this might be fine; many
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'm open to skipping it as it was a small potential improvement, considering the concerns raised.
Co-authored-by: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com>
As suggested by Sid in #712
As suggested by Sid in #712
This adds support to spec-generator for generating Bikeshed specs and issue lists, in a way that provides near-parity with the features of the CSSWG API.
Functionality included:
die-on)md-date)md-prepare-for-tr)md-<key>)The most notable thing that isn't included is an equivalent to the CSSWG API's
output=err, which I'd like to work on separately after this, to avoid making this PR even bigger.The diff will be rather large, because I moved the majority of the code that had been in
server.tsintogenerator/respec.tsin the process, as a lot of it was respec-specific. Nowgenerator/bikeshed.tsandgenerator/respec.tseach export a similar API whichserver.tscalls after validating parameters, depending on thetypeparameter.Other changes included in this PR:
die-onsupport to ReSpec, to maximize the amount of shared fields between the available generatorsmd-dateandmd-<key>support to ReSpec, to make it possible to overriderespecConfigproperties even in case of file upload, which previously wasn't possiblestartfunction inserver.tsasync, to wait until the server is listeningindex.htmlto 2-space indent (from 4) to match other repos and default Prettier settings; since this PR results in a lot of changed lines already due to most code being added or relocated, it made sense to tackle this now as well (already discussed with Denis)index.htmlthrough Prettier in the process, but ended up re-excluding it because of a couple of non-configurable decisions Prettier makes that I disagree with, primarily the inclusion of self-closing tagsnpm run watchto also restart on HTML changesnpm testto correctly run all tests - and there are many more now, including tests across GET, POST, and POST with query string parameters, wherever applicable