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

concepts: workspace #2453

Merged
merged 28 commits into from
Aug 4, 2021
Merged

concepts: workspace #2453

merged 28 commits into from
Aug 4, 2021

Conversation

jorgeorpinel
Copy link
Contributor

@jorgeorpinel jorgeorpinel commented May 10, 2021

Continues/addresses #2197

Added workspace.md doc to basic-concepts dir. Expanded this from previous iterations.

Added Basic Concepts section to the sidebar and added this document.

tooltip definition is not changed.

This PR closes #1127 (comment) and is related to #550.

See https://dvc-org-concept-workspa-4abjdl.herokuapp.com/doc/user-guide/basic-concepts


@jorgeorpinel jorgeorpinel changed the title concept: workspace concepts: workspace May 10, 2021
@shcheklein shcheklein temporarily deployed to dvc-org-concept-workspa-4abjdl May 10, 2021 03:04 Inactive
@jorgeorpinel jorgeorpinel self-assigned this May 10, 2021
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-concept-workspa-4abjdl May 11, 2021 03:33 Inactive
@jorgeorpinel jorgeorpinel marked this pull request as ready for review May 11, 2021 04:58
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-concept-workspa-4abjdl May 11, 2021 04:58 Inactive
@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented May 11, 2021

Cc @iesahin in case you want to check out what I had in mind for this one or have any comments.

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-concept-workspa-4abjdl May 12, 2021 14:57 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-concept-workspa-4abjdl May 12, 2021 14:57 Inactive
@jorgeorpinel
Copy link
Contributor Author

@jorgeorpinel
Copy link
Contributor Author

Ok this is stale. Need to plan releasing several basic concepts in tandem... Closing for now

@jorgeorpinel
Copy link
Contributor Author

Reopening per #2595 (comment)

@jorgeorpinel jorgeorpinel reopened this Jul 8, 2021
@shcheklein shcheklein temporarily deployed to dvc-org-concept-workspa-6kl6xs July 8, 2021 03:37 Inactive
Comment on lines 2 to +7
name: Workspace
match: [workspace]
tooltip: >-
Directory containing all your project files e.g. raw datasets, source code, ML
models, etc. Typically, it's also a Git repository. It will contain your DVC
project.
Directory containing all your DVC project files, e.g. raw data, source code,
ML models. One project version at a time is visible in the workspace.
See full [concept page](/doc/user-guide/basic-concepts/workspace).
Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Jul 21, 2021

Choose a reason for hiding this comment

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

Hi @julieg18 and @rogermparent: Is there a way to add a link property to the tooltip frontmatter structures (maybe afer tooltip)? So it links from here:

image

Instead of having to add a sentence with the link to the tooltip text (as in the code above). Thanks.

This comment was marked as resolved.

Copy link
Contributor

@rogermparent rogermparent Jul 24, 2021

Choose a reason for hiding this comment

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

I would think we'd want to automatically know the link and populate it without manually specifying it, but either way (or both if we want some override behavior) should work. @julieg18 may have more details, since she built out this feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

automatically know the link and populate it without manually specifying it

OK but the tricky part is that it should only link to the corresponding user-guide/basic-concepts/<name> page IF there's MD content in this same file (under the frontmatter). For now a manual link field is good enough and it can become the override later if we implement the auto-linking after that.

So should we make that change in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Co-assigned with @julieg18 for now 🙂

Copy link
Contributor

@julieg18 julieg18 Jul 26, 2021

Choose a reason for hiding this comment

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

It should be possible to implement this! We can update the glossary.tsx to check for a link field and decide what to render. Do we want the "Workspace" to still have the same styling as the other tooltip titles(black with an icon that appears on hover) or do we want it to look like a link(blue with underline on hover)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Should look like a link but I guess also bold since it's the entry's title? Up to you

@jorgeorpinel jorgeorpinel removed the request for review from shcheklein July 25, 2021 20:04
@casperdcl
Copy link
Contributor

casperdcl commented Jul 28, 2021

ah so https://dvc-org-concept-workspa-6kl6xs.herokuapp.com/doc/user-guide/basic-concepts/workspace renders fine (even though it's not in the ToC nav). Could address that (1. adding an auto-generated Basic Concepts nav entry, and/or 2. linking from the Glossary entries) in a follow-up engine PR.

@jorgeorpinel
Copy link
Contributor Author

basic-concepts/workspace renders fine (even though it's not in the ToC nav

Yep. So for now no need to add menu entries at least until several pages are available.

@julieg18 I'll link from the glossary texts for now and open a separate issue to auto-link from the names. Thanks

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-concept-workspa-6kl6xs August 4, 2021 16:15 Inactive
@jorgeorpinel
Copy link
Contributor Author

p.s. another reason to have a Concepts menu or index besides linking from the Glossary is the order in which they're listed e.g. Workspace should be the among first as it's pretty basic but in the glossary is the last due to the automatic alphabetical order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

term: ambiguous use of "external" and "workspace"
6 participants