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

link/term: .dvc/ (dir) & "internal" #2120

Merged
merged 10 commits into from
Jan 30, 2021
Merged

link/term: .dvc/ (dir) & "internal" #2120

merged 10 commits into from
Jan 30, 2021

Conversation

jorgeorpinel
Copy link
Contributor

@jorgeorpinel jorgeorpinel commented Jan 27, 2021

Continues #2098 and #2117

@jorgeorpinel jorgeorpinel changed the title guide: better links and other improvements to Project Structure guide guide: better links and other improvements to Project Structure Jan 27, 2021
@shcheklein shcheklein temporarily deployed to dvc-landing-2-0-pipelin-xkejno January 27, 2021 23:08 Inactive
@jorgeorpinel jorgeorpinel changed the title guide: better links and other improvements to Project Structure guide: better links and other impros to Project Structure Jan 27, 2021
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-xkejno January 27, 2021 23:17 Inactive
## Generating multiple stages
## Foreach stages
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does everyone agree on this term? Cc @dberenbaum

Copy link
Member

Choose a reason for hiding this comment

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

foearch should be code-inlined I think ... There was an option like "Something (foreach)" - I would personally prefer that one

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't find any suggestion like "Something (foreach)" but I'm fine with "foreach stages"

Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Jan 30, 2021

Choose a reason for hiding this comment

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

I think the "Something" was "Loop" but I argued we should avoid that direct reference.

Went for quoted foreach stages — a minor issue is how it looks in the ToC nav:

image

UPDATE: This change is now in #2124.

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-xkejno January 27, 2021 23:27 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-xkejno January 27, 2021 23:32 Inactive
@jorgeorpinel jorgeorpinel marked this pull request as ready for review January 27, 2021 23:32
Comment on lines 15 to 16
matches: '.dvcignore',
url: '/doc/user-guide/project-structure/dvcignore-files'
Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Jan 27, 2021

Choose a reason for hiding this comment

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

Hi @rogermparent 👋

For some reason this new autolink is not being picked up in https://dvc-landing-2-0-pipelin-xkejno.herokuapp.com/doc/user-guide/project-structure.

It works everywhere in https://dvc-landing-2-0-pipelin-xkejno.herokuapp.com/doc/command-reference/add and https://dvc-landing-2-0-pipelin-xkejno.herokuapp.com/doc/command-reference/check-ignore.

I noticed that if the bullet doesn't start with `.dvcignore` then it works. But that's not a problem for the previous bullets which start with `dvc.yaml` and `.dvc` 🤷

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like it's being picked up to me. Maybe it was a cache issue?

Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Jan 30, 2021

Choose a reason for hiding this comment

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

It's still happening on my local even after rm -rf .cache/ but I see it's OK in the review app now 🤷

UPDATE: Extracted this to #2125.

@shcheklein
Copy link
Member

PR again addressed to much stuff at once

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Jan 27, 2021

Yeah sorry, #2098 was a significant change that required all these link updates. And while reviewing those, better options started to emerge (like the auto-links). I didn't plan all the check-boxes in advance, just added them as they came.

@shcheklein
Copy link
Member

@jorgeorpinel 👍 but let's try to this as 4 PRs please :) it's very hard to review this.

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-xkejno January 30, 2021 03:41 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-xkejno January 30, 2021 04:07 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-xkejno January 30, 2021 04:15 Inactive
jorgeorpinel added a commit that referenced this pull request Jan 30, 2021
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-xkejno January 30, 2021 04:23 Inactive
shcheklein pushed a commit that referenced this pull request Jan 30, 2021
…ge (#2124)

* glossary: finish DVC file tooltips
extracted from e8b56f0

* term: multi-stages -> `foreach` stages
extracted per #2120 (comment)
@jorgeorpinel jorgeorpinel changed the title guide: better links and other impros to Project Structure link/term: .dvc/ (dir) & "internal" Jan 30, 2021
@jorgeorpinel
Copy link
Contributor Author

let's try to this as 4 PRs please

Done @shcheklein. Order: #2124, this one, #2125

@jorgeorpinel jorgeorpinel merged commit bb68104 into master Jan 30, 2021
@jorgeorpinel jorgeorpinel deleted the 2.0/pipelines branch February 1, 2021 06:43
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.

4 participants