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.

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

guide: reorg sidebar #4011

Merged
merged 7 commits into from
Oct 4, 2022
Merged

guide: reorg sidebar #4011

merged 7 commits into from
Oct 4, 2022

Conversation

dberenbaum
Copy link
Collaborator

Proposed restructure of the guide based on discussion at the bottom of #144. I only updated the sidebar so we can iterate on that before propagating the changes elsewhere.

@shcheklein shcheklein temporarily deployed to dvc-org-ug-reorg-zcajwdgw5kgni September 30, 2022 19:35 Inactive
@github-actions
Copy link
Contributor

github-actions bot commented Sep 30, 2022

d328427

Link Check Report

There were no links to check!

CML watermark

"slug": "data-management",
"children": [
"large-dataset-optimization",
"defining-pipelines",
Copy link
Member

Choose a reason for hiding this comment

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

The only thing that raises some concern from my end. It should not be in the data management to my mind. I think pipelines deserve their own place in the UG. It's just content should be very different as we were discussing in the PR for the pipelines.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I struggled to keep them separate because:

  1. I wasn't sure where to put external-dependencies (and managing-external-data/external outputs to a lesser extent) since they are about external data but only really make sense in the context of pipelines.
  2. Pipelines in general is tightly coupled with data management now (I would like to address this on the product side, but it's not going to happen this year).
  3. From a use case perspective, I can see an argument for keeping them together (a typical data engineering workflow needs data versioning and pipelines).

Still, I'm happy to keep pipelines separate and agree it seems like a more natural structure (see my comments at the bottom of #144). Where would you suggest to put external-dependencies and managing-external-data?

Copy link
Member

Choose a reason for hiding this comment

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

since they are about external data but only really make sense in the context of pipelines.

Hmm, it think the both could be used independently though? Especially external outputs (like versioning an external directory).

Even if that is the case, then we should move them into pipelines, out of the data management.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What about something like this (not sure how to format it correctly):

      {
        "label": "Data Management",
        "slug": "data-management",
        "children": [
            "large-dataset-optimization",
            {
              "label": "Managing External Data",
              "slug": "managing-external-data"
            }
        ]
      },
      {
        "label": "Pipelines",
        "slug": "pipelines",
        "children": [
            "defining-pipelines",
            "external-dependencies",
            {
              "label": "External Outputs",
              "slug": "external-outputs",
              "source": "/docs/user-guide/managing-external-data"
            }
        ]
      },

Copy link
Member

Choose a reason for hiding this comment

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

Yep, sounds reasonable to start with. We'll need to move imports I think into data management as a next step then.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, I didn't catch your point about imports before. In that case, we can duplicate both sections:

      {
        "label": "Data Management",
        "slug": "data-management",
        "children": [
            "large-dataset-optimization",
            {
              "label": "Importing External Data",
              "source": "/docs/user-guide/pipelines/external-dependencies"
            },
            {
              "label": "Managing External Data",
              "slug": "managing-external-data"
            }
        ]
      },
      {
        "label": "Pipelines",
        "slug": "pipelines",
        "children": [
            "defining-pipelines",
            "external-dependencies",
            {
              "label": "External Outputs",
              "source": "/docs/user-guide/data-management/managing-external-data"
            }
        ]
      },

Copy link
Member

Choose a reason for hiding this comment

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

yep, don't know if that is possible though :) (would be a nice feature it is possible)

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's ideal to have page duplicates like that. I'd consider it a patch for either a product or docs content issue that should be resolved.

Copy link
Member

@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.

Good stuff, I would reconsider the pipelines part. Otherwise LGTM (maybe we need some redirects)

@shcheklein shcheklein temporarily deployed to dvc-org-ug-reorg-zcajwdgw5kgni October 4, 2022 00:36 Inactive
@dberenbaum dberenbaum marked this pull request as ready for review October 4, 2022 00:36
redirects-list.json Outdated Show resolved Hide resolved
Comment on lines +160 to 166
"running-dvc-on-windows",
"setup-google-drive-remote",
"stop-tracking-data",
"update-tracked-data",
"add-deps-or-outs-to-a-stage",
"resolve-merge-conflicts",
"share-a-dvc-cache"
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor (could be a follow-up) in terms of order and page naming:

Suggested change
"running-dvc-on-windows",
"setup-google-drive-remote",
"stop-tracking-data",
"update-tracked-data",
"add-deps-or-outs-to-a-stage",
"resolve-merge-conflicts",
"share-a-dvc-cache"
"run-dvc-on-windows",
"stop-tracking-data",
"update-tracked-data",
"resolve-merge-conflicts",
"add-deps-or-outs-to-a-stage",
"setup-google-drive-remote",
"share-a-dvc-cache"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TBH I think they're fine as is. For example, I intentionally put windows and google drive at the top since they are blockers for doing almost anything for those users.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK but (minor)

Comment on lines 148 to +149
"comparing-experiments",
"visualizing-plots",
Copy link
Contributor

Choose a reason for hiding this comment

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

Possibly follow-up

Should the Parallel Coordinates Plot section be moved or linked from the Visualizing Plots page?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think it should be moved since it's more related to comparing experiments, but linking could make sense.

redirects-list.json Outdated Show resolved Hide resolved
@shcheklein shcheklein temporarily deployed to dvc-org-ug-reorg-zcajwdgw5kgni October 4, 2022 06:55 Inactive
Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

A bunch of link updates should follow this. Fun fun... Lmk if you need help!

p.s. I think this would basically close #144 ? (finally)

Comment on lines +127 to +128
"importing-external-data",
"managing-external-data"
Copy link
Contributor

Choose a reason for hiding this comment

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

An alternative to consider (from #144 (comment)) which may be better since we don't love the product feature as-is now (I think):

External Deps/Managing External Data: ... or make them only reachable via links.

Copy link
Contributor

@jorgeorpinel jorgeorpinel Oct 4, 2022

Choose a reason for hiding this comment

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

p.s. should also update the H1 in importing-external-data.md

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

or make them only reachable via links.

I like it better than having no solution for using data directly from the cloud, so I'm going to keep it in.

@jorgeorpinel jorgeorpinel added A: docs Area: user documentation (gatsby-theme-iterative) p1-important Active priorities to deal within next sprints C: guide Content of /doc/user-guide labels Oct 4, 2022
@dberenbaum
Copy link
Collaborator Author

A bunch of link updates should follow this. Fun fun... Lmk if you need help!

I guess it's not that bad, but it seems like the redirects should make it a pretty low priority, right?

An auto update script when adding redirects would be 🔥 .

@shcheklein shcheklein temporarily deployed to dvc-org-ug-reorg-zcajwdgw5kgni October 4, 2022 17:33 Inactive
@jorgeorpinel
Copy link
Contributor

the redirects should make it a pretty low priority

Not sure. Will we remember what links to update if we don't do it soon?

auto update script

Prob overkill: we mainly need to run find/replace per link throughout content/docs 🙂

@shcheklein shcheklein temporarily deployed to dvc-org-ug-reorg-zcajwdgw5kgni October 4, 2022 17:53 Inactive
@dberenbaum
Copy link
Collaborator Author

Good feedback @jorgeorpinel and @shcheklein! If one of you want to do one more pass, I'll merge if everything looks good.

jorgeorpinel added a commit that referenced this pull request Oct 4, 2022
@jorgeorpinel
Copy link
Contributor

LGTM. I started another PR for the links: #4015

@shcheklein shcheklein temporarily deployed to dvc-org-ug-reorg-zcajwdgw5kgni October 4, 2022 19:53 Inactive
@dberenbaum dberenbaum merged commit d328427 into main Oct 4, 2022
@dberenbaum dberenbaum deleted the ug-reorg branch October 4, 2022 20:20
jorgeorpinel added a commit that referenced this pull request Oct 14, 2022
* guide: reorg sidebar

* guide: update sidebar plus redirects

* Update redirects-list.json

* guide: minor fixes to sidebar edits

* plots: more links for pcp
rel #4011 (review)

* fix admon typoe

* guide: update links (#4017)

* guide: update links based on
https://github.com/iterative/dvc.org/pull/4011/files#diff-a6828b19e9794c1fbb6799ea95263fb7487b676d054932b5f6e3b315eb12a6a2

* start: remove Data Mgmt index page (#4016)

* start: remove Data Mgmt index pg and
move some of its contents to the GS index page

* Update content/docs/start/index.md

* dvc 2.29.0 (#4021)

Co-authored-by: efiop <[email protected]>

* how-to: fix run-dvc-on-win links

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: efiop <[email protected]>

* Update src/components/DownloadButton/index.tsx

Co-authored-by: dberenbaum <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: efiop <[email protected]>
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: guide Content of /doc/user-guide p1-important Active priorities to deal within next sprints
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants