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

Remove learn/legacy and add redirect to /learn #20643

Merged
merged 1 commit into from Feb 15, 2018
Merged

Conversation

caleybrock
Copy link
Contributor

On average about 15 people a day are viewing this page so Tanya and I agree it can be removed.

@@ -0,0 +1 @@
/learn
Copy link
Member

Choose a reason for hiding this comment

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

Can this be a .moved file since it's a permanent move?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought .moved was for pages where we take the same page and move it to another place, where as in this case we are just deprecating the page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From will:
pretty sure the .moved/.redirect stuff is a legacy/undocumented feature of pegasus (https://github.com/code-dot-org/pegasus/commit/088d1f543c0289c5e1d080cb9ee1b18e02f5eee0), so i don't have any extra rule of thumb to give there. Apparently .moved and .redirect do the same thing, and .redirect was added as an alias for .moved per hadi? (https://github.com/code-dot-org/pegasus/commit/4e4599abbf2b626bfd31743406f3dbf34030f20c)
.found does a 302 temporary redirect, .moved and .redirect to 301 permanent redirects.


= view 'mobilecheck.js'

= view :learn_teacher_banner
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this is the only file exclusively used by this file, so good job catching that it's okay for deletion.

Copy link
Contributor

@balderdash balderdash left a comment

Choose a reason for hiding this comment

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

💀

@caleybrock caleybrock merged commit 3d12816 into staging Feb 15, 2018
@caleybrock caleybrock deleted the remove-learn-legacy branch February 15, 2018 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants