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.

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

Support for redirects other than the old site ones. #156

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Support for redirects other than the old site ones. #156

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
96f2723
Support for redirects other than the old site ones.
benjyw Feb 17, 2024
79b5749
Rename redirects file
benjyw Feb 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions docusaurus.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import versions from "./versions.json";
import redirects from "./old_site_redirects.js";
import redirects from "./redirects.js";
import old_site_redirects from "./old_site_redirects.js";
import captionedCode from "./src/remark/captioned-code.js";
import tabBlocks from "docusaurus-remark-plugin-tab-blocks";
import fs from "fs";
Expand Down Expand Up @@ -369,7 +370,7 @@ const config = {
[
"@docusaurus/plugin-client-redirects",
{
redirects,
redirects: old_site_redirects.concat(redirects),
},
],
],
Expand Down
10 changes: 10 additions & 0 deletions redirects.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export default [
Copy link
Member

Choose a reason for hiding this comment

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

Can we rename this to make it more obvious what kinds of redirects these are? Something like renamed_path_redirects?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, but I'm also concerned about the larger question of the fact that CI can't pass on this until the new pages exist in the repo, which requires publishing, and then re-publishing so the redirects go live.

Copy link
Member

Choose a reason for hiding this comment

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

I think that's a good thing. It's catching bad redirects and won't publish.

I think the correct thing to do is to add the infra in this PR, and then update it in the docs sync PR when we sync renamed paths

Copy link
Contributor Author

@benjyw benjyw Feb 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

How would that work? Isn't that an automatic PR during release?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a good thing, but it means that there is an impedance mismatch between how docs publishing works and implementing redirects.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, whoever would've been reviewing that PR would have to do it.

We could move that file into the pants repo and special-case "sync"-ing it.

But then, we might wanna invest in automation on making sure the pants repo "builds" the docs healthily

Copy link
Contributor

Choose a reason for hiding this comment

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

I think having this file in the main repo and syncing it naively/wholesale would be hard to manage due to cherry-picks. For instance, we're adding a redirect here for 2.19 only:

  • If we put the redirect file update into the main PR, then a release/doc sync from main won't work if that happens before the 2.19.x changes are in
  • If we put it into the 2.19 cherry pick PR, it will be overridden by the next main sync.

(I imagine we can make the automation more intelligent to solve this, but that means more complexity too.)

I wonder if we should have live chat about our approach to documentation and releases so we can get to shared direction more quickly rather than trying to hash it out over text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I was pondering this, and I think redirects are fundamentally not a versioned thing.

{
from: "/2.19/docs/java-and-scala",
to: "/2.19/docs/jvm/java-and-scala",
},
{
from: "/2.19/docs/java-and-scala/kotlin",
to: "/2.19/docs/jvm/kotlin",
},
];
Loading