Create a bot that adds branch-snapshot links to PRs about meta HOT 26 CLOSED

I'm looking at hosting PRs in an S3 bucket.

from meta.

@foolip do you mean that as a general problem with our branch-snapshots/ idea? I don't think that's problematic as we only care about branch-snapshots while making the change. If you want to refer back to a change you should look at commit-snapshots/.

from meta.

Yes. The bot's here: https://github.com/tobie/webidl-pr-preview

It's hosted on heroku.

I've spent time today making it more generic.

I now need to handle repositories which build the spec through travis, which I'll need to do for WebIDL anyway.

At that point, I should be able to offer it as a service running on the same Heroku instance for all Bikeshed specs.

from meta.

Oh, neato! Note that our URLs would be of the form https://url.spec.whatwg.org/branch-snapshots/clarify-shorten/, i.e. a different one per PR. And it would only apply to PRs that are "in repo" i.e. not from forks.

Alternately I suppose it could try to use the Bikeshed web service. See e.g. the preview link at WebBluetoothCG/web-bluetooth#355 . That won't work great for WHATWG specs out of the box since they require some environment variables. But then it would work for all PRs, including those from forks.

Anyway I think probably the easiest way to go is for me to hack on it until we have something that works for WHATWG specs. I'll try to keep an eye on genericness while doing so. Then I can PR against the original.

from meta.

I've built in support for setting non private env variables (use a .pr-preview.json file in your repo, see bikeshed_parameters), so that shouldn't be an issue.

from meta.

Oh, I see, these env vars are dynamic.

Maybe we can figure something out, here.

from meta.

Are we cool with publishing fork PRs in the same origin?

from meta.

Alright, so the S3 hosting is all setup. We'll need to exchange some keys to get this working elsewhere. LMK who would be available to do that and when. (Preferably over skype/hangouts.)

from meta.

We've set this up for URL. @tobie you'll ping us when you're ready for more?

from meta.

OK, you're good to go. Instructions are here. There's now a helper for the config file.

You can do a blanket install on the whole org, as the presence of the config file is now what triggers the preview.

Note you might consider creating dedicated Bikeshed boilerplate to avoid needing so much info in the config file.

from meta.

Note you might consider creating dedicated Bikeshed boilerplate to avoid needing so much info in the config file.

Would you mind explaining that a bit more?

from meta.

Yeah, sorry. I thought the warnings were specific to the WHATWG include files, it turns out they're common Bikeshed boilerplate, so maybe this should be handled at the Bikeshed level.

My thinking around this was simply that we're going to be creating a slew of files that are PR snapshots and that really shouldn't be used as the basis of anything, so we might as well clearly mark them as such. And possibly do so in a way that's not as obtrusive as the modal dialogues.

Copying @marcoscaceres and @tabatkins as they might have thoughts on this wrt to Bikeshed and ReSpec respectively (namely, how can we easily mark-up PR previews so they're not confused with ED/WD/LS and ideally link them back to the PR).

from meta.

And possibly do so in a way that's not as obtrusive as the modal dialogues.

This is quite intentional, we don't want anyone to get confused with respect to the status of those documents.

from meta.

Anyway, it sounds like the boilerplate JSON resource should simply be like the URL Standard, right?

from meta.

This is quite intentional, we don't want anyone to get confused with respect to the status of those documents.

Yeah, they have class=annoying-warning for a reason.

(namely, how can we easily mark-up PR previews so they're not confused with ED/WD/LS and ideally link them back to the PR).

Happy to add a new warning category with a more specific message. And come to think of it, I should really add a "custom" warning that can be specified right in the metadata, too.

from meta.

This is quite intentional, we don't want anyone to get confused with respect to the status of those documents.

Yeah, they have class=annoying-warning for a reason.

I guess. Maybe a "I know what I'm doing, fold those modal warnings away" cookie would be a nice addition.

from meta.

That's just an attractive nuisance, tho - nobody likes the warnings, so they'll fold them away, then they'll get caught by a seemingly-warningless document!

from meta.

Created a couple of PRs to start. Didn't get through the entire WHATWG alphabet just yet.

from meta.

I have them all now apart from console/streams/html-differences.

from meta.

Will something terrible happen if two different PRs use the same branch name, or is there some scheme to append -1 and so on to avoid that?

from meta.

Will something terrible happen if two different PRs use the same branch name, or is there some scheme to append -1 and so on to avoid that?

@foolip: wouldn't both these PRs be identical, though?

from meta.

@tobie, AFAIK there's nothing that enforces the branch names to be unique over time. One PR can use the name "fix-typo", the branch is removed when the PR is merged, and then created again for another PR later.

from meta.

If you want to refer back to a change you should look at commit-snapshots/.

Note that, at some point, I'm planning to add, in a <details> element, a link to a preview of every commit in the PR along with the HTML diff with its parent commit.

from meta.

Hmm, I simply assumed that the links were intended to keep working indefinitely, since they're about the first thing a spec archeologist will find to click. If the links are removed from the comments or replaced by links to the merged commit, there would be no problem.

from meta.

@foolip can you file a bug on https://github.com/tobie/pr-preview so I don't forget about it?

from meta.

I'm going to close this since we have a system in place now. We'll be fiddling with the system and it might have bugs, but we really ought to track those separately as this is getting rather long. So please file new issues if you encounter something.

from meta.