-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
chore(rebuild): re-incorporate redirects #2034
Conversation
1d8bffb
to
7ea523c
Compare
What happened to the redirects.json file? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great to me and I love how we've isolated redirects to a single location. The only concern I'd have is how redirects are handled within the site. Meaning that because these are server-side files that will be output, once you've entered the site and are in SPA mode, these won't be hit.
Now that I think about it though, maybe that's a non-issue. Any links we have control over should just be updated, and these are mainly for external sites linking back here. Once the conflict is addressed I think this is good to go.
DISCUSSION.md
Outdated
@@ -13,7 +13,7 @@ this branch: | |||
- [ ] Extract anchors into `_content.json` via `DirectoryTreePlugin` | |||
- [ ] Finish re-incorporating mobile sidebar | |||
- [ ] Re-integrate google-analytics | |||
- [ ] Re-incorporate `redirects.json` | |||
- [x] Re-incorporate `redirects.json` (Eugene) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you drop this change to fix the conflict? I removed DISCUSSION.md
in favor of github threads (i.e. PRs and issues). It wasn't meant to be a long-lived document anyway, just gave some perspective on various issues.
When the hyperlink branch merges in you won't have any links pointing at moved pages any more. I'm also pretty sure we already addressed all of them on master, but if there are any left, rest assured you'll have automation to find them |
7ea523c
to
658e5d6
Compare
@skipjack its done |
Including this plugin in `common` causes it to be run twice for production builds as both parallelized configs pick this up. We only need it in `prod` and it's kind of nice to have all html generation happen in one build so lumping it together with the `SSGPlugin` makes sense.
afce5bf
to
bd2beca
Compare
@EugeneHlushko hope you don't mind, I tested this out before merging and noticed that plugin was actually run twice because of our parallelized I'm going to merge, but if you see any issue with this change don't hesitate to comment. |
Re-intergrate redirects
Note: i took latest redirects list from
master