Skip to content

Well-known routes #207

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Nov 9, 2015
Merged

Well-known routes #207

merged 5 commits into from
Nov 9, 2015

Conversation

dirk
Copy link
Contributor

@dirk dirk commented Nov 3, 2015

Implementation of #197.

Things to discuss:

  • Possible CSRF action if a crate has a malicious documentation URL?
  • This currently transitions to the crate's main page if there isn't a documentation URL; is that the behavior we want? (Update: No, showing a brief flash message before redirecting.)
  • Should we take this opportunity to add more well-known paths? (@durka suggested homepage and repository in their original issue.)

@durka
Copy link
Contributor

durka commented Nov 3, 2015

Awesome!
On Nov 3, 2015 12:48 AM, "Dirk Gadsden" [email protected] wrote:

Implementation of #197
#197.
Things to discuss:

  • Possible CSRF action if a crate has a malicious documentation URL?
  • This currently transitions to the crate's main page if there isn't a
    documentation URL; is that the behavior we want?
  • Should we take this opportunity to add more well-known paths? (@durka
    https://github.com/durka suggested homepage and repository in their
    original issue.)

You can view, comment on, or merge this pull request online at:

#207
Commit Summary

  • Fix loader.js Bower dependency
  • Add .env to make local Foreman use more pleasant
  • Add 'docs' well-known route for getting to a crate's documentation

File Changes

Patch Links:

—
Reply to this email directly or view it on GitHub
#207.

@dirk dirk force-pushed the dirk/well-known-routes branch from 4c74a77 to c5788db Compare November 3, 2015 06:22
@dirk dirk force-pushed the dirk/well-known-routes branch from c5788db to 2a75aca Compare November 3, 2015 06:37
@steveklabnik
Copy link
Member

is that the behavior we want?

I would suggest at least a flash message.

@dirk
Copy link
Contributor Author

dirk commented Nov 4, 2015

@steveklabnik: Done! Here's a little preview of it when the documentation URL is present:
 
image

@alexcrichton
Copy link
Member

This looks pretty good to me, thanks @dirk! I agree with others in that an instantaneous transition is probably the best way to go here, the page can still say "redirecting to ..." but scheduling the redirect ASAP seems best.

I also agree with @steveklabnik that a missing doc URL should probably just emit a flash message and redirect back to the crate itself.

@dirk
Copy link
Contributor Author

dirk commented Nov 5, 2015

Cool, thanks for you guise input! I'll get this implemented today. (Sorry for not implementing this yesterday, was a bit under the weather.)

@dirk
Copy link
Contributor Author

dirk commented Nov 6, 2015

@durka, @steveklabnik: Revised UX is implemented. 😄

@alexcrichton
Copy link
Member

OK, looks good to me, thanks @dirk!

alexcrichton added a commit that referenced this pull request Nov 9, 2015
@alexcrichton
Merge pull request #207 from dirk/dirk/well-known-routes
9add27e 
Well-known routes
@alexcrichton alexcrichton merged commit 9add27e into rust-lang:master Nov 9, 2015
@dirk
Copy link
Contributor Author

dirk commented Nov 9, 2015

@alexcrichton: Yup, glad to help! 🎉

@dirk dirk deleted the dirk/well-known-routes branch November 9, 2015 19:33
@Turbo87 Turbo87 mentioned this pull request Nov 9, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dirk @durka @steveklabnik @alexcrichton @Turbo87