docs: add redirects for docs IA update #388
Conversation
👷 Deploy request for es-eslint pending review.Visit the deploys page to approve it
|
✅ Deploy Preview for new-eslint ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
eslint-github-bot
bot
added
triage
documentation
✅ Deploy Preview for zh-hans-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for fr-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for hi-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for ja-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for de-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for pt-br-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
This doesn't seem to work. For example, https://deploy-preview-388--new-eslint.netlify.app/docs/latest/user-guide/ does not redirect to https://deploy-preview-388--new-eslint.netlify.app/docs/latest/use/
Perhaps the new redirect rules should be before this one so that the new rules match first:
eslint.org/src/static/redirects.njk
Lines 34 to 35 in 52f9eb5
https://docs.netlify.com/routing/redirects/#rule-processing-order
didn't realize that there was a staging site that redirects would work on here. thx for call out. will play around w that for testing |
ok, implemented the suggested change, and it's working as expected. using example above, now https://deploy-preview-388--new-eslint.netlify.app/docs/latest/user-guide/ redirects to https://deploy-preview-388--new-eslint.netlify.app/docs/latest/use/ it also captures the splat patterns such as https://deploy-preview-388--new-eslint.netlify.app/docs/latest/user-guide/page-does-not-exist to <https://deploy-preview-388--new-eslint.netlify.app/docs/latest/use/page-does-not-exist with that being said, there's a lot of conditional logic going on in this file that i don't fully understand, so it'd be great if this could be reviewed taking that into account. |
Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
|
The conditional logic is primarily for the various translation sites, only one of which (zh-hans.eslint.org) has its own docs. Although that does remind me: when we are renaming files, we need to make sure to do it for the zh-hans docs as well or else everything will break. |
ok, how do you think is best to proceed with this? i don't know chinese so am probably not too useful with that. |
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
|
For the start, we could add these redirects only for the English site. Once zh-hans.eslint.org is ready for this change, we can remove the condition. |
|
|
|
|
|
|
|
If we'd like to enable these redirects only for the English site ( |
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
@mdjermanovic would you mind putting suggestions for this change on the PR? i don't fully understand what's going on with conditionals statements for the redirects |
|
@mdjermanovic thank you again for a thorough review. added the above suggested redirects and also went through the all file moves in the eslint/eslint#16665 and added some redirects that were covered by the splat to be explicit in f5e151a |
There was a problem hiding this comment.
@mdjermanovic would you mind putting suggestions for this change on the PR
Here it is
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
|
If we are doing this just for the English site now, then maybe we should put these into the English site file instead? I've been trying to avoid checking the language of the site inside of other files. |
I think it would be easier to leave this here, as it's only temporary. Once we update the |
|
Sounds good 👍 |
There was a problem hiding this comment.
LGTM, thanks!
This should be merged right after the release that includes eslint/eslint#16665
btmills
dismissed
nzakas’s stale review
Feedback appears to have been addressed, and this PR needs to be merged after the release.
Prerequisites checklist
What is the purpose of this pull request?
Redirects for docs site as part of the information architecture overhaul.
Should be merged together with eslint/eslint#16665
What changes did you make? (Give an overview)
Added redirects for new docs information architecture.
Related Issues
eslint/eslint#16665
Is there anything you'd like reviewers to focus on?
Note: created some helper scripts in this repo - https://github.com/bpmutter/eslint-redirects
This can (but doesn't have to be) used to make any bulk changes to the additions here.