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

Make /install work under FastBoot #1912

Merged
merged 3 commits into from
Dec 3, 2019
Merged

Conversation

kzys
Copy link
Contributor

@kzys kzys commented Nov 22, 2019

Will add nginx.conf change after #1907. Early feedback is welcome!

@rust-highfive
Copy link

r? @carols10cents

(rust_highfive has picked a reviewer for you, use r? to override)

@jtgeibel
Copy link
Member

What do you think about converting app/routes/crate/repo.js and app/routes/crate/docs.js in this PR as well? It seems like the _redirectTo logic could be shared between these routes.

(app/routes/github-login.js sets window.location as well, but supporting login without JS is a whole other thing, if we decide to support that use case.)

@kzys
Copy link
Contributor Author

kzys commented Nov 23, 2019

Sure. I can do that. But please deploy/enable /policies change first. We should not enable three or four FastBoot-enabled routes at once, especially since we don't have any yet.

@jtgeibel
Copy link
Member

But please deploy/enable /policies change first. We should not enable three or four FastBoot-enabled routes at once, especially since we don't have any yet.

I agree, we should enable that first. That change is deployed now, but I don't have the ability to enable it on production, and I can't speak for @carols10cents on when that will be possible.

kzys added 3 commits November 25, 2019 20:57
The service can be used from other routes that modify window.location.
That makes the routes work under FastBoot.
@jtgeibel
Copy link
Member

jtgeibel commented Dec 3, 2019

Looks good to me!

This should be safe to deploy at any time, as no changes are made to nginx.

@bors r+

@bors
Copy link
Contributor

bors commented Dec 3, 2019

📌 Commit 976e8e8 has been approved by jtgeibel

@bors
Copy link
Contributor

bors commented Dec 3, 2019

⌛ Testing commit 976e8e8 with merge 8f56f5f...

bors added a commit that referenced this pull request Dec 3, 2019
Make /install work under FastBoot

Will add nginx.conf change after #1907. Early feedback is welcome!
@bors
Copy link
Contributor

bors commented Dec 3, 2019

☀️ Test successful - checks-travis
Approved by: jtgeibel
Pushing 8f56f5f to master...

@bors bors merged commit 976e8e8 into rust-lang:master Dec 3, 2019
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.

5 participants