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

Meta: add HTML page for pull request redirects #1882

Closed
wants to merge 1 commit into from
Closed

Conversation

bakkot
Copy link
Contributor

@bakkot bakkot commented Feb 25, 2020

This adds an HTML page for redirecting to the netlify snapshot of the spec on each PR, and a link in the pull request template to that page.

I am pretty confident that the HTML itself works, but I'm not confident that it will get copied to the right place. Unfortunately there's no good way to test it short of landing this PR.

Obviously this link won't work yet, but it will look like:

View the specification with this PR included.

If it doesn't work, you can get there yourself by clicking the "Show all checks" at the bottom of the pull request, then clicking the "details" link next to the "netlify/ecma262-snapshots/deploy-preview" check, as in <a href="https://cdn.netlify.com/f0b93f3003d5669924b6e73540bbfe07bc3d8d95/34c5d/img/blog/deploy-preview-workflow.gif">this animation</a>.
</p>
<script>
var match = /^https:\/\/github\.com\/tc39\/ecma262\/pull\/(\d+)/.exec(document.referrer);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, Safari's ITP is gonna break this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we made a little script that edits the proper link into the pr description?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This blog post says this only happens when "the referrer has link decoration", which I think shouldn't be a problem here (since github URLs do not tend to be decorated). Is that not the whole picture?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devsnek I'm not really a fan of editing other people's comments, even with a bot, but I guess that would be acceptable if this doesn't work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s more recent: https://webkit.org/blog/9661/preventing-tracking-prevention-tracking/

We could just have a bit that comments on the PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm definitely not a fan of a comment bot.

If Safari's going to break this, this probably isn't a viable approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well that sucks.

Opened #1883 for further discussion. I'm going to close this for now.

@bakkot bakkot closed this Feb 25, 2020
@ljharb ljharb deleted the pr-redirect branch February 25, 2020 04:36
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

5 participants