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

Adds "preview" status, for @tobie previews #1334

Merged
merged 5 commits into from Jul 27, 2017
Merged

Conversation

marcoscaceres
Copy link
Member

  • makes specs unofficial when previewing

@marcoscaceres
Copy link
Member Author

@tobie, this now will set things to unofficial and add a warning if the spec status is "unofficial". When you generate the spec, can you please add ?specStatus=unofficial to the URL.

@tobie
Copy link
Member

tobie commented Jul 26, 2017

Might also be worth adding a link back to the PR, etc.

@tobie
Copy link
Member

tobie commented Jul 26, 2017

OK, shipped and deployed this. LMK how it goes?

@tobie
Copy link
Member

tobie commented Jul 26, 2017

Oh, wait! Do you want specStatus=preview or specStatus=unofficial?

@marcoscaceres
Copy link
Member Author

specStatus=preview

@tobie
Copy link
Member

tobie commented Jul 26, 2017

Fixed.

@tobie
Copy link
Member

tobie commented Jul 26, 2017

This seems to be breaking specs which now reference missing resources on w3.org, e.g.: https://s3.amazonaws.com/pr-preview/w3c/webrtc-pc/issue-1470-patch.html

@marcoscaceres
Copy link
Member Author

@tobie thanks for trying it out. Upon reflection, I think what I actually want here is only to signal that it's a preview (?isPreview=true), which will add the ugly red-annoying-box but not change the status.

It occurred to me me that Keeping the spec status is useful for checking snapshots against PubRules etc. I'll update the PR and ping you to change things again on your side (to append isPreview=true on your end). Appreciate your time and help with this!

@tobie
Copy link
Member

tobie commented Jul 26, 2017

Added isPreview=true.

@marcoscaceres marcoscaceres merged commit a7530b4 into develop Jul 27, 2017
@marcoscaceres marcoscaceres deleted the preview_status branch July 27, 2017 03:15
@marcoscaceres
Copy link
Member Author

@tobie, I'll implement the branch thing separately. Might need to chat to you about it in IRC. There are a few things that could work, like:

?isPreview=true&github={"branch": "branch", repoURL: "https://github.com/w3c/repo"}

Then I can key off github there to provide the link back, similar the WHATWG specs. However, I don't know how useful that is, as people don't generally browse branches (maybe I'm missing something).

@tobie
Copy link
Member

tobie commented Jul 27, 2017

I was mostly just thinking about having a link back to the PR. Useful in case you land on it and want the broader context and/or the merge status.

@tobie
Copy link
Member

tobie commented Jul 27, 2017

You can get that easily from the config object:

{
    "params": {
        "prUrl": "{{ pull_request.html_url }}",
        "prNumber": "{{ pull_request.number }}"
    }
}

So it's trivial to add them to extra/default params in the URL.

@marcoscaceres
Copy link
Member Author

oh, nice! Ok, would need to add those to ReSpec - but would be trivial.

@tobie
Copy link
Member

tobie commented Jul 27, 2017

Well, LMK which ones you'll need. :)

@marcoscaceres
Copy link
Member Author

@tobie will do! Will try to hack on it next week.

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

Successfully merging this pull request may close these issues.

None yet

2 participants