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

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

Open
tabatkins opened this issue Jul 24, 2023 · 4 comments
Open

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

tabatkins opened this issue Jul 24, 2023 · 4 comments

Comments

@tabatkins
Copy link

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:

  • just mark the macro as optional, with [COMMIT-SHA?]. If it's not set it'll silently be replaced with nothing.
  • set the macro in the whatwg defaults to the dummy value.

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

@domenic
Copy link
Member

domenic commented Jul 25, 2023

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

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

jgraham commented Jul 26, 2023

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

jgraham commented Jul 26, 2023

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants