Skip to content

Fix redirects if latest version failed to build #509

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

Merged
merged 3 commits into from
Feb 23, 2020

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Dec 7, 2019

Closes #502

The redirector behaved differently for /:crate/:version/:target than
for /:crate/:version/:target/. This changes the latest version link to
use the former, since it properly accounts for failed builds.

Additionally, this changes the /:target redirect to keep the query
parameter so that ?search= URLs will be kept when going to /:target/.

r? @QuietMisdreavus

Some good crates to test this with:

  • isatty 0.1.0, 0.2.0 (0.2.0 failed to build)
  • pyo3 0.2.7, 0.8.2 (moved pyo3::exc::ArithmeticError to a different module)

@QuietMisdreavus
Copy link
Member

So, to double check what this PR does:

  • Changes the "jump to latest version" redirect to not have a trailing slash before adding the search query
  • Changes the /:crate//:crate/:version redirect to include a query string in the redirect.

Would it be possible to check for the presence of the query string and only include the ? in the URL if it's already there? I get a little annoyed when URLs have an unnecessary ? or # sometimes.

@jyn514
Copy link
Member Author

jyn514 commented Dec 12, 2019

Yes, that's right. I can change it to remove the '?' if there's no query, give me a second.

@jyn514
Copy link
Member Author

jyn514 commented Dec 12, 2019

Ok should be fixed.

@jyn514

This comment has been minimized.

@jyn514 jyn514 force-pushed the failed-build-search branch from e72902f to e662e56 Compare January 31, 2020 05:01
@jyn514

This comment has been minimized.

@jyn514 jyn514 self-assigned this Jan 31, 2020
@jyn514

This comment has been minimized.

@jyn514 jyn514 removed their assignment Feb 11, 2020
@jyn514 jyn514 self-assigned this Feb 19, 2020
Closes rust-lang#502

The redirector behaved differently for `/:crate/:version/:target` than
for `/:crate/:version/:target/`. This changes the latest version link to
use the former, since it properly accounts for failed builds.

Additionally, this changes the `/:target` redirect to keep the query
parameter so that `?search=` URLs will be kept.
- add query parameter if it exists
- don't call `env.frontend()` a dozen times
- add tests
@jyn514 jyn514 force-pushed the failed-build-search branch from e662e56 to 1934869 Compare February 19, 2020 05:32
@jyn514
Copy link
Member Author

jyn514 commented Feb 19, 2020

This is ready for review.

@jyn514
Copy link
Member Author

jyn514 commented Feb 20, 2020

r? @kvrhdn

@yvrhdn
Copy link

yvrhdn commented Feb 20, 2020

Sure @jyn514, I should be able to check out these changes this weekend 🙂

Copy link

@yvrhdn yvrhdn left a comment

Choose a reason for hiding this comment

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

Looks good to me, don't have any comments 🙂

@jyn514 jyn514 merged commit 22a3ddf into rust-lang:master Feb 23, 2020
@jyn514 jyn514 deleted the failed-build-search branch February 23, 2020 14:49
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.

Redirect to latest version does not work if build failed
3 participants