Review Drafts automation

[annevk]

I looked into this a bit and one problem is that copy_extra_files and run_post_build_step only really work on Travis and not for a local deploy. Those should be part of a Review Draft however. And we can't really check in the source and let Travis generate the output since we want that to be done once and commit the output.

So we could try to read .travis.yml locally (perhaps not that hard, even in bash?) or revamp the way we do this.

[annevk]

Also: REVIEW_DIR="review-drafts/$(date +'%Y-%m')" is what we want I think. Yields review-drafts/2018-04 if run today.

[domenic]

Hmm it'd be better to check in the source text and let Travis generate the output if possible... That is, review-drafts/2018-05.bs gets checked in with the source as of that moment in time.

We'd need some way to make it so that it doesn't get re-built every time, but only each time we add a new review draft or update some minor text inside the review draft. That seems pretty doable; we'd be able to use git to see if the file has changed.

[annevk]

I guess it is indeed good to keep the source around, but we should be very clear that the source only works for generating the output for a limited time, as we might change the tooling around and I wouldn't want to have to support decade-old source files forever.

How would you see Travis generating the output? It generates it and then commits it on the PR branch? And we somehow prevent that triggering another round (unless a further commit changes the source). I guess that could work with some research. We'd also need to ensure that further changes don't rejigger the date (if we really wanted to change the date we'd open a new PR).

[domenic]

as we might change the tooling around and I wouldn't want to have to support decade-old source files forever.

Agreed.

How would you see Travis generating the output? It generates it and then commits it on the PR branch?

I was thinking just the same way as we do everything else: it generates it then uploads it to the server.

The tricky part is just making it not do that if we haven't changed the source. But it seems doable.

We'd also need to ensure that further changes don't rejigger the date (if we really wanted to change the date we'd open a new PR).

Bikeshed has a Date metadata that you can set explicitly, thankfully.

[annevk]

git diff --name-status $TRAVIS_COMMIT_RANGE seemed to be what we need to know the difference between added, modified, and not touched. However, $TRAVIS_COMMIT_RANGE is reportedly problematic: travis-ci/travis-ci#2668.

https://developer.github.com/v3/pulls/#list-pull-requests-files might therefore be a better fit. We could process such a response in bash with something akin to https://github.com/nest/nest-simulator/pull/75/files. https://stedolan.github.io/jq/manual/ has the manual for jq.

Apart from that we need some client-side instruction that copies the source and puts it in the correct directory. This probably fits in the Makefile and we'd just copy-and-paste.

[domenic]

(Status update: I have #200 and speced/bikeshed#1239 ready; #201 is in progress and we can both push to that as we figure things out here.)

I think git diff --name-status HEAD~1 would suffice. It would not detect changes if we merged a PR that included changes to a review draft, but no such changes were present in the last commit. In such a case we could manually trigger the missing build however. I'm tempted to just do this? But I can kind of imagine someone five years from now tearing out their hair and wondering why things aren't working because of this edge case.

It's a little unclear to me whether $TRAVIS_COMMIT_RANGE is broken for our use cases, but that thread doesn't look too promising, so maybe not worth risking it...

Are you excited about working on the jq thing? Otherwise maybe do git diff --name-status HEAD~1 for now?

As for workflow, I have written down:

• Editor copies dom.bs into review-drafts/2018-05.bs
• Editor adds Date: today's date to the top of the file.
• Editor commits and pushes. Review draft is automatically generated and deployed to https://dom.spec.whatwg.org/review-drafts/2018-05/
• Editor keeps working on master, changing dom.bs. Pushing commits that touch dom.bs does not re-build or deploy the review draft.
• Some kind of minor text update needs to happen to the review draft for whatever reason. Editor touches review-drafts/2018-05.bs and commits it.
• Pushing then re-generates and re-deploys to https://dom.spec.whatwg.org/review-drafts/2018-05/.
  • If this fails, e.g. because the tooling has gotten ahead of the source files, then the deploy doesn't happen, and we figure out what to do (e.g. not bother with this minor text update).

The first two steps could be put into a Makefile, indeed.

[tabatkins]

Editor adds Date: today's date to the top of the file.

Do they also have to change to Status: RD, or is that automatically overridden for them by the build process?

[domenic]

Yeah, automatically passed in for any files inside the review-drafts/ directory by the deploy script.

[annevk]

This took me way too long, but this is the way to get a Review Draft in bash:

#!/bin/bash
set -o errexit
set -o nounset

mkdir -p "review-drafts"
INPUT="url.bs"
REVIEW_DRAFT="review-drafts/$(date +'%Y-%m').bs"
# The very unusual way of creating a newline here is done to keep macOS happy
cp "$INPUT" temp
sed 's/^Group: WHATWG$/&\'$'\n'"Date: $(date +'%Y-%m-%d')/g" temp > "$REVIEW_DRAFT"
rm temp
echo "Created Review Draft at $REVIEW_DRAFT"
echo "Differences with $INPUT:"
diff -up "$INPUT" "$REVIEW_DRAFT"

I tried to figure out how to do this in a Makefile, but no luck.

Okay to wrap this in a curl call as well from the Makefile and host this next to deploy.sh as review.sh?

[annevk]

I'd propose we simplify Makefile as well to just:

deploy: url.bs
	curl --remote-name --fail https://resources.whatwg.org/build/deploy.sh && bash ./deploy.sh
review: url.bs
	curl --remote-name --fail https://resources.whatwg.org/build/review.sh && bash ./review.sh

Encouraging folks to install bikeshed and keep it up-to-date generally isn't a great use of their time. It's easier to rely on the server for the couple times you want to check something locally and not in a PR.

[annevk]

(If we use this approach change the INPUT line to INPUT=$(find . -maxdepth 1 -name "*.bs" -print -quit) as per deploy.sh.)

[domenic]

Yeah, Makefiles have their own weird not-quite-bash syntax. I think you can embed bash in them, but it'd be easier to just check in a script like you say. Let's remember to add that script to .gitignore.

I'd like to keep the local target in the Makefile, as I myself use it pretty often when contributing to the various specs. But I'm fine deemphasizing it in the contribution docs.

Similarly we should keep the remote target because making 3 curl calls to Bikeshed, like the deploy.sh script does, is much slower than just making one such call, as the Makefile does.

[annevk]

It is, but remote doesn't catch a number of things such as linting and such. Anyway, I'm fine with just adding to the Makefiles for now and maybe simplifying over time as more moves into Bikeshed (such as fatal warnings and errors).