whatwg / meta

Discussions and issues without a logical home
Creative Commons Zero v1.0 Universal
93 stars 161 forks source link

Make `[COMMIT-SHA]` macro less annoying locally? #286

Open tabatkins opened 11 months ago

tabatkins commented 11 months ago

If you run bikeshed directly on any WHATWG spec, you get a fatal error about a COMMIT-SHA macro not being set. This comes from the group boilerplate, so it's not apparent what's actually wrong from looking at the spec itself. The intended practice is that you build with make local, which just invokes bikeshed with that text macro supplied with a dummy value.

It would be nice to lower friction here and allow plain bikeshed invocation. There are two easy fixes we could choose from:

Either solution still allows the macro to be set to its correct value by the deploy.sh script, as it does today.

domenic commented 11 months ago

Running bikeshed locally on WHATWG specs is not recommended, because bikeshed is too error-tolerant by default, and we don't want to allow that.

If there was an additional ability to require die-on=warning behavior via inline metadata or similar, then I'd be less worried about the consequences here.

But overall I think this is over-indexing on one person's complaint that they don't like make, whereas many other people appreciate having a uniform cross-platform build system so they don't need to know "in this repo you run bikeshed spec, in this repo you run whatever ReSpec command, in this repo you run Wattsi + html-build, in this repo you run npm install && npm run build".

jyasskin commented 11 months ago

I also stumble over the need to run make local on WHATWG specs, so it's not just one person.

I love Domenic's idea of putting die-on=warning into spec metadata, and I'd use it for all of my specs.

jgraham commented 11 months ago

Note that make local in the standard WHATWG Makefile is not in fact passing --die-on=warning. That makes sense because for local development it's not actually that helpful; it's easier to debug problems if you can see the output of the parts that did work. This is especially true in the not-infrequent case where a warning is introduced by third party changes (e.g. new references that introduce ambiguity into the base commit of a local branch).

On the other hand it totally makes sense that every spec would have CI which enforces warnings as errors.

Anyway this seems like a distraction from the main goal here which is that just running bikeshed spec should work for WHATWG specs, which would have the side effect of allowing users to trivially provide whichever command line arguments they want.

jgraham commented 11 months ago

Maybe bikeshed would benefit from bikeshed spec --ci which would enable all the recommended flags for running in CI. That would make it easier to setup the actual CI systems, and make it easy to do a "would this build pass CI" check locally. For extra marks the spec metadata could allow customising the default behaviour for this mode (assuming there's some use case for not all specs behaving identically).