Respec: improve isPreview text

[marcoscaceres]

See:
https://github.com/w3c/respec/pull/1334#issuecomment-318301008

Can add bit of magic, and allow linking back to pull request.

[OrionStar25]

Can I work on this @marcoscaceres ?

[marcoscaceres]

Sure, but I can't recall exactly what would need to be done. Can you take a look at the referenced bug/discussion above and make a proposal here? Once we get agreement on what we should do, then we can code it up.

[OrionStar25]

This is something which was initially wanted:
https://s3.amazonaws.com/pr-preview/rmisev/url/empty-host-for-file.html

Along with this:

• [ ] a link to the concerned pr/commit and
• [ ] the status of the spec
  needs to be shown too. All this was so that we can see what major changes have been made in the versions.

This is our current isPreview :

[marcoscaceres]

Feel free to have a go at trying to put something together. I'm still not sure clear as to what configuration options need to be added, but maybe you can prototype something while I'm away.

[yatri1609]

@marcoscaceres I would like to take this issue up for the next week

[marcoscaceres]

@yatri1609 sounds great. When you can, take a look at
https://github.com/w3c/respec/pull/1334#issuecomment-318301008 ... as I can't recall the details. Suggestions on how to make things better are welcomed!

[yatri1609]

Sure I'll suggest what I can do on this and discuss here. @marcoscaceres

[yatri1609]

@marcoscaceres So I checked by clicking the preview so it does reverts back to this screen so we can just add to the title that This is a preview of the PR #xyz solving Issue #abc

What do you think?

[marcoscaceres]

If we can get both those bits of information, that would be amazing. Just having “this is a preview of pull request X” would be already be a big improvement.

[tobie]

Yeah, these are easy to get. And you can give the config file default values here: https://github.com/tobie/pr-preview/blob/master/lib/models/config.js#L36

[yatri1609]

Yeah I'll surely do that

[yatri1609]

@tobie As you said so just to clarify if I'm doing it right or wrong, I need to add .pr-preview.json file right with the params I need to pass, and define things out there? Am I doing it right or is it something that I have miissunderstood? Thanks

[tobie]

I’m not sure I understand your question. Some of that data is dynamic, so it can’t belong in pr-preview.json, further more you want something generic so not every spec has to be changed for this to work.

[yatri1609]

I got it now! Thanks

[marcoscaceres]

@yatri1609 if you see any other goodies from https://github.com/tobie/pr-preview we could use, please feel free to suggest improvements to the current text. If the params just get added to the URL, then we should be able to pick them up easily from the config object.

@tobie, thanks again for your guidance!

[tobie]

Now you should open a PR against PR Preview itself to get that data passed by default. Would be silly to request this from all config files.

[yatri1609]

Now you should open a PR against PR Preview itself to get that data passed by default. Would be silly to request this from all config files.

I didn't understand what are actually telling me to do.

[sidvishnoi]

I didn't understand what are actually telling me to do.

Send a PR to @tobie/pr-preview to edit config.js for respec.

We need to add those params (prNumber and prUrl) in defaults, so users don't have to explicitly specify them in their pr-preview.json files.

[yatri1609]

Ohk I got you! I thought that but then I again got confused!

[tobie]

Yeah, sorry for the terse comments. I’ve had little time dor this but wanted to make sure that this wasn’t forgotten.

[yatri1609]

Yeah, sorry for the terse comments. I’ve had little time dor this but wanted to make sure that this wasn’t forgotten.

No problem @tobie