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

Jump to bottom

[Ready] Rename docs to remove underscores, fix resulting broken links #321

Merged
merged 16 commits into from
Feb 21, 2024
Merged

[Ready] Rename docs to remove underscores, fix resulting broken links #321

merged 16 commits into from
Feb 21, 2024

Conversation

stichbury
Copy link
Contributor

@stichbury stichbury commented Feb 20, 2024 •
edited

Description

This PR changes names as #313 describes. Docs are building as expected (see https://vizro-ai--321.org.readthedocs.build/en/321/ for example) and all link warnings are now resolved.

HOWEVER, there's a linter issue I'm struggling to resolve.

Screenshot

Notice

  • I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

    • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
    • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
    • I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.
    • I have not referenced individuals, products or companies in any commits, directly or indirectly.
    • I have not added data or restricted code in any commits, directly or indirectly.

@stichbury
Change prettier settings
58bb8b9 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Remove underscores
cb585a3 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Add docs to gitignore
770e65c 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Fix broken internal links
520a581 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Fix broken links
d686f97 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury stichbury added the Component: Docs 🗒️ Issue/PR for markdown and API documentation label Feb 20, 2024
@stichbury stichbury self-assigned this Feb 20, 2024
stichbury and others added 3 commits February 20, 2024 11:51
@stichbury
Fix broken links
dd375f7 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Add changelog
db2d92d 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
e355f55 
for more information, see https://pre-commit.ci
stichbury and others added 2 commits February 20, 2024 12:25
@stichbury
Update vizro-core/src/vizro/models/types.py
9b1ac36 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
ca1072a 
for more information, see https://pre-commit.ci
@stichbury stichbury marked this pull request as ready for review February 20, 2024 13:56
@stichbury stichbury changed the title Rename docs to remove underscores, fix resulting broken links [Ready] Rename docs to remove underscores, fix resulting broken links Feb 20, 2024
huong-li-nguyen
huong-li-nguyen approved these changes Feb 20, 2024
View reviewed changes
Copy link
Contributor

@huong-li-nguyen huong-li-nguyen left a comment •
edited

Choose a reason for hiding this comment

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

LGTM 👍

I think you need to add another changelog file under vizro-core! You've already done so for vizro-ai, but because you have changes in vizro-core as well, you need to add a second changelog file there!

Just run this command from the directoy of vizro-core: hatch run changelog:add

I've proposed a code change below to fix the remaining linting issue :)

vizro-core/docs/pages/user-guides/tabs.md Outdated Show resolved Hide resolved
vizro-core/src/vizro/models/types.py Outdated Show resolved Hide resolved
stichbury and others added 5 commits February 20, 2024 14:04
@stichbury @huong-li-nguyen
Update vizro-core/docs/pages/user-guides/tabs.md
f145764 
Co-authored-by: Li Nguyen <90609403+huong-li-nguyen@users.noreply.github.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury @huong-li-nguyen
Update vizro-core/src/vizro/models/types.py
4ac0c72 
Co-authored-by: Li Nguyen <90609403+huong-li-nguyen@users.noreply.github.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
35dd30b 
for more information, see https://pre-commit.ci
@stichbury
add changelog for vizro-core
d6aff5a 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Merge branch 'second-branch-docs-name-changes' of https://github.com/…
16f2e4b 
…mckinsey/vizro into second-branch-docs-name-changes
@huong-li-nguyen huong-li-nguyen added the Status: Ready for Review ☑️ Issue/PR is ready for review - all tests have passed label Feb 20, 2024
maxschulz-COL
maxschulz-COL approved these changes Feb 20, 2024
View reviewed changes
Copy link
Contributor

@maxschulz-COL maxschulz-COL left a comment

Choose a reason for hiding this comment

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

Lgtm - would remove the changelog entries (not the files ;) )

vizro-ai/changelog.d/20240220_115353_jo_stichbury_second_branch_docs_name_changes.md Outdated Show resolved Hide resolved
vizro-core/changelog.d/20240220_140357_jo_stichbury_second_branch_docs_name_changes.md Outdated Show resolved Hide resolved
@stichbury
Roll back content in changelog files
18d4038 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Copy link
Contributor Author

Here is a list of the pages I changed -- when we make the next release we should decide which of these should be redirected in the RTD console (to help people with stored links or blog posts etc that link to the old _ names).


vizro-ai/docs/pages/explanation/safety_in_vizro_ai.md
--> vizro-ai/docs/pages/explanation/safety-in-vizro-ai.md
vizro-ai/docs/pages/tutorials/explore_vizro_ai.md
--> vizro-ai/docs/pages/tutorials/explore-vizro-ai.md
vizro-ai/docs/pages/user_guides/api_setup.md
--> vizro-ai/docs/pages/user-guides/api-setup.md
vizro-ai/docs/pages/user_guides/install.md
--> vizro-ai/docs/pages/user-guides/install.md
vizro-ai/docs/pages/user_guides/model_config.md
--> vizro-ai/docs/pages/user-guides/model-config.md
vizro-ai/docs/pages/user_guides/run_vizro_ai.md
--> vizro-ai/docs/pages/user-guides/run-vizro-ai.md

vizro-core/docs/pages/API_reference/actions.md
--> vizro-core/docs/pages/API-reference/actions.md
vizro-core/docs/pages/API_reference/manager.md
--> vizro-core/docs/pages/API-reference/manager.md
vizro-core/docs/pages/API_reference/models.md
--> vizro-core/docs/pages/API-reference/models.md
vizro-core/docs/pages/API_reference/vizro.md
--> vizro-core/docs/pages/API-reference/vizro.md
vizro-core/docs/pages/explanation/why_vizro.md
--> vizro-core/docs/pages/explanation/why-vizro.md
vizro-core/docs/pages/tutorials/explore_components.md
--> vizro-core/docs/pages/tutorials/explore-components.md
vizro-core/docs/pages/tutorials/first_dashboard.md
--> vizro-core/docs/pages/tutorials/first-dashboard.md
vizro-core/docs/pages/user_guides/actions.md
--> vizro-core/docs/pages/user-guides/actions.md
vizro-core/docs/pages/user_guides/assets.md
--> vizro-core/docs/pages/user-guides/assets.md
vizro-core/docs/pages/user_guides/card_button.md
--> vizro-core/docs/pages/user-guides/card-button.md
vizro-core/docs/pages/user_guides/components.md
--> vizro-core/docs/pages/user-guides/components.md
vizro-core/docs/pages/user_guides/container.md
--> vizro-core/docs/pages/user-guides/container.md
vizro-core/docs/pages/user_guides/custom_actions.md
--> vizro-core/docs/pages/user-guides/custom-actions.md
vizro-core/docs/pages/user_guides/custom_charts.md
--> vizro-core/docs/pages/user-guides/custom-charts.md
vizro-core/docs/pages/user_guides/custom_components.md
--> vizro-core/docs/pages/user-guides/custom-components.md
vizro-core/docs/pages/user_guides/custom_tables.md
--> vizro-core/docs/pages/user-guides/custom-tables.md
vizro-core/docs/pages/user_guides/dashboard.md
--> vizro-core/docs/pages/user-guides/dashboard.md
vizro-core/docs/pages/user_guides/data.md
--> vizro-core/docs/pages/user-guides/data.md
vizro-core/docs/pages/user_guides/filters.md
--> vizro-core/docs/pages/user-guides/filters.md
vizro-core/docs/pages/user_guides/graph.md
--> vizro-core/docs/pages/user-guides/graph.md
vizro-core/docs/pages/user_guides/install.md
--> vizro-core/docs/pages/user-guides/install.md
vizro-core/docs/pages/user_guides/integration.md
--> vizro-core/docs/pages/user-guides/integration.md
vizro-core/docs/pages/user_guides/layouts.md
--> vizro-core/docs/pages/user-guides/layouts.md
vizro-core/docs/pages/user_guides/navigation.md
--> vizro-core/docs/pages/user-guides/navigation.md
vizro-core/docs/pages/user_guides/pages.md
--> vizro-core/docs/pages/user-guides/pages.md
vizro-core/docs/pages/user_guides/parameters.md
--> vizro-core/docs/pages/user-guides/parameters.md
vizro-core/docs/pages/user_guides/run.md
--> vizro-core/docs/pages/user-guides/run.md
vizro-core/docs/pages/user_guides/selectors.md
--> vizro-core/docs/pages/user-guides/selectors.md
vizro-core/docs/pages/user_guides/table.md
--> vizro-core/docs/pages/user-guides/table.md
vizro-core/docs/pages/user_guides/tabs.md
--> vizro-core/docs/pages/user-guides/tabs.md
vizro-core/docs/pages/user_guides/themes.md
--> vizro-core/docs/pages/user-guides/themes.md

@antonymilne
Copy link
Contributor

@stichbury is there any harm in putting all those redirects in place right now? So that:

  • we're not blocking a release waiting for it
  • we can start updating links already, e.g. in my post on the plotly forums

Also, as per my comment in #313, I'm very happy to just have the most important redirects if adding all of them is more effort than it's worth.

@stichbury
Copy link
Contributor Author

@stichbury is there any harm in putting all those redirects in place right now?

I'm not sure -- we could put in some redirects for latest I suppose but we'd have to adjust them at release anyway, since otherwise we end up redirecting everything (including stable) to latest and probably don't want to do that?

@stichbury
Copy link
Contributor Author

Merging this anyway, we can figure out redirects separately!

@stichbury stichbury merged commit 06e9824 into main Feb 21, 2024
34 checks passed
@stichbury stichbury deleted the second-branch-docs-name-changes branch February 21, 2024 09:46
@stichbury
Copy link
Contributor Author

stichbury commented Feb 21, 2024 •
edited

Redirects for pages are the priority, in order of most-least :

  • /pages/tutorials/first_dashboard/
  • /pages/tutorials/explore_components/
  • /pages/explanation/why_vizro/
  • /pages/API_reference/models/
  • /pages/user_guides/table/
  • /pages/API_reference/vizro/
  • /pages/user_guides/layouts/
  • /pages/user_guides/run/
  • /pages/user_guides/custom_charts/
  • /pages/user_guides/pages/
  • /pages/user_guides/components/
  • /pages/user_guides/data/
  • /pages/user_guides/custom_components/
  • /pages/user_guides/navigation/
  • /pages/user_guides/assets/
  • /pages/user_guides/custom_actions/
  • /pages/user_guides/actions/
  • /pages/user_guides/install/
  • /pages/API_reference/actions/
  • /pages/user_guides/integration/
  • /pages/user_guides/selectors/
  • /pages/user_guides/custom_tables/
  • /pages/API_reference/manager/
  • /pages/user_guides/tabs/
  • /pages/user_guides/card_button/
  • /pages/user_guides/container/
  • /pages/user_guides/dashboard/
  • /pages/user_guides/filters/
  • /pages/user_guides/graph/
  • /pages/user_guides/parameters/
  • /pages/user_guides/themes/

@stichbury stichbury mentioned this pull request Feb 21, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxschulz-COL maxschulz-COL maxschulz-COL approved these changes

@huong-li-nguyen huong-li-nguyen huong-li-nguyen approved these changes

@Joseph-Perkins Joseph-Perkins Awaiting requested review from Joseph-Perkins Joseph-Perkins is a code owner

@antonymilne antonymilne Awaiting requested review from antonymilne antonymilne is a code owner

@Anna-Xiong Anna-Xiong Awaiting requested review from Anna-Xiong Anna-Xiong is a code owner

Assignees

@stichbury stichbury

Labels
Component: Docs 🗒️ Issue/PR for markdown and API documentation Status: Ready for Review ☑️ Issue/PR is ready for review - all tests have passed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@stichbury @antonymilne @maxschulz-COL @huong-li-nguyen