Skip to content

get-started: initial refactoring#1051

Merged
jorgeorpinel merged 16 commits into
masterfrom
refactor/get-started
Mar 20, 2020
Merged

get-started: initial refactoring#1051
jorgeorpinel merged 16 commits into
masterfrom
refactor/get-started

Conversation

@jorgeorpinel
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel commented Mar 15, 2020

@shcheklein shcheklein temporarily deployed to dvc-landing-refactor-ge-nouj86 March 15, 2020 19:59 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-refactor-ge-nouj86 March 15, 2020 23:06 Inactive
@jorgeorpinel
Copy link
Copy Markdown
Contributor Author

jorgeorpinel commented Mar 15, 2020

@shcheklein so far this only implements #747 (creates https://dvc-landing-refactor-ge-nouj86.herokuapp.com/doc) but probably a little hacky – see getItemByPath in src/utils/sidebar.js and SidebarMenu.timeout in src/components/Documentation/SidebarMenu/index.js. Can you guys look at it and lmk what a better way to implement this could be? Cc @fabiosantoscode

Also if this should be a PR of it's own I can separate it from the remaining changes (Get Started related).

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-refactor-ge-nouj86 March 15, 2020 23:17 Inactive
@shcheklein
Copy link
Copy Markdown
Contributor

Looks good to me! We should spend a bit of time to think about the landing page content (check other projects). Also, see this discussion that is relevant to your changes in the sidebar.json - #1025 (comment) ... cc @iAdramelk - if we can merge this one before Gatsby migration we might just disallow having any entry w/o index page? But to be honest I would still do redirects if file is missing, and we should be doing this on the middleware level automatically (not relying on sidebar.json).

@shcheklein

This comment has been minimized.

@jorgeorpinel
Copy link
Copy Markdown
Contributor Author

jorgeorpinel commented Mar 16, 2020

  • We should spend a bit of time to think about the landing page content (check other projects)

Working on that ⏳

we need to introduce some special "Home" entry to the sidebar?

I don't think so, we have the DOC link in the top menu. But if you prefer that then sure, that wouldn't require the js code changes I did.

if we can merge this one before Gatsby migration

If you want to merge this PR soon, please approve it and I'll limit it to this first step for now.

@shcheklein

This comment has been minimized.

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-refactor-ge-nouj86 March 16, 2020 00:59 Inactive
@jorgeorpinel

This comment has been minimized.

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-refactor-ge-nouj86 March 16, 2020 01:51 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-refactor-ge-nouj86 March 16, 2020 03:24 Inactive
@jorgeorpinel

This comment has been minimized.

@shcheklein

This comment has been minimized.

@shcheklein

This comment has been minimized.

@shcheklein
Copy link
Copy Markdown
Contributor

shcheklein commented Mar 16, 2020

@shcheklein

This comment has been minimized.

@jorgeorpinel

This comment has been minimized.

@treeverse treeverse deleted a comment from shcheklein Mar 16, 2020
@jorgeorpinel jorgeorpinel changed the title get-started: big refactoring get-started: initial refactoring Mar 17, 2020
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-refactor-ge-nouj86 March 18, 2020 19:49 Inactive
@jorgeorpinel

This comment has been minimized.

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-refactor-ge-nouj86 March 18, 2020 21:02 Inactive
@jorgeorpinel jorgeorpinel requested a review from shcheklein March 18, 2020 21:02
@jorgeorpinel

This comment has been minimized.

Comment thread redirects-list.json Outdated
Comment thread redirects-list.json
"^/doc/?$ /doc/get-started 307",
"^/doc/user-guide/contributing/?$ /doc/user-guide/contributing/core 307",
"^/doc/understanding-dvc/?$ /doc/understanding-dvc/collaboration-issues 307",
"^/doc/changelog/?$ /doc/changelog/0.18 307",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I really want to get rid of changelog, make it an external link ... again a special case like a Home button ... we spend time on it again and again

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I thought it was an external link already before? I didn't introduce this in the Gatsby migration, just moved the order of some redirects.

Comment thread src/components/Community/data.json Outdated
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-refactor-ge-nouj86 March 19, 2020 07:37 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-refactor-ge-nouj86 March 19, 2020 07:40 Inactive
@jorgeorpinel jorgeorpinel mentioned this pull request Mar 19, 2020
5 tasks
@jorgeorpinel jorgeorpinel requested a review from shcheklein March 19, 2020 07:47
@jorgeorpinel
Copy link
Copy Markdown
Contributor Author

Let's try to address the remaining refactoring in #1074 (or further PRs)? Any problems specific to these nav order changes I can still fix here, of course.

Copy link
Copy Markdown
Contributor

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

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

Looks good to me! Make sure that all pages we move we make redirects from their previous locations. Feel free to merge!

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-refactor-ge-nouj86 March 20, 2020 21:18 Inactive
@jorgeorpinel jorgeorpinel merged commit 859763c into master Mar 20, 2020
@jorgeorpinel jorgeorpinel deleted the refactor/get-started branch March 21, 2020 03:01
@jorgeorpinel jorgeorpinel added A: docs Area: user documentation (gatsby-theme-iterative) p1-important Active priorities to deal within next sprints C: start Content of /doc/start labels Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A: docs Area: user documentation (gatsby-theme-iterative) C: start Content of /doc/start p1-important Active priorities to deal within next sprints

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: move /get-started index to root and /install above /get-started

2 participants