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

docs nav RFC: move pipes to concepts #20712

Merged
merged 5 commits into from
May 7, 2024
Merged

Conversation

yuhan
Copy link
Contributor

@yuhan yuhan commented Mar 25, 2024

Summary & Motivation

this PR moves pipes pages to Concepts

  • updates all internal links
  • updates nav -- open to suggestions to the exact order of the nav
  • sets up redirects so /guides/dagster-pipes URLs out there would still work

How I Tested These Changes

preview: https://03-25-docs-move-pipes-to-concepts.dagster.dagster-docs.io/concepts/dagster-pipes

image

@yuhan yuhan requested a review from erinkcochran87 as a code owner March 25, 2024 17:13
Copy link
Contributor Author

yuhan commented Mar 25, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @yuhan and the rest of your teammates on Graphite Graphite

Copy link

github-actions bot commented Mar 25, 2024

Deploy preview for dagster-docs ready!

Preview available at https://dagster-docs-2iado1p71-elementl.vercel.app
https://03-25-docs-move-pipes-to-concepts.dagster.dagster-docs.io

Direct link to changed pages:

@yuhan yuhan changed the title docs: move pipes to concepts docs nav RFC: move pipes to concepts Mar 25, 2024
@yuhan yuhan requested review from slopp and schrockn March 25, 2024 17:26
Copy link
Contributor

@slopp slopp left a comment

Choose a reason for hiding this comment

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

lgtm - didnt do extensive QA but did click around a bit

maybe overtime this can move from advanced to a core asset concept (I could see it displacing some combo of external assets / asset observations)

@yuhan
Copy link
Contributor Author

yuhan commented Mar 25, 2024

maybe overtime this can move from advanced to a core asset concept (I could see it displacing some combo of external assets / asset observations)

yes thats my thinking too. but i dont think we are there today, maybe soon tho.

@schrockn
Copy link
Member

Do we have a general principle here for moving something from "guide" to "concept"? Just want to have a policy here so we have a rationale going forward.

@yuhan yuhan force-pushed the 03-25-docs_move_pipes_to_concepts branch from ac9ba79 to 514769c Compare May 2, 2024 21:39
@yuhan yuhan force-pushed the 03-25-docs_move_pipes_to_concepts branch from 64c65a6 to 765aee2 Compare May 2, 2024 21:54
@yuhan
Copy link
Contributor Author

yuhan commented May 2, 2024

Do we have a general principle here for moving something from "guide" to "concept"? Just want to have a policy here so we have a rationale going forward.

My own take is that we started with "guide" because we were unsure of where this would live and we didn't want to introduce yet another "core concept" without any real adoption/PMF (in other words, this would have existed in a notion page if not for testability). Now that we've seen sufficient adoption and interests to adopt, we are confident that this would be a "concept" (a core abstraction) that exists in a long run. So now it makes sense to move it to "concept". cc @erinkcochran87 for general docs guidelines if any.


also cc @erinkcochran87 for another quick review pass to make sure my rebase on this old PR doesn't break any recent doc changes.

Copy link
Contributor

@erinkcochran87 erinkcochran87 left a comment

Choose a reason for hiding this comment

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

This looks good to me. Tested a bunch of links and redirects and didn't hit any errors, so I think that part is fine.

Do we have a general principle here for moving something from "guide" to "concept"? Just want to have a policy here so we have a rationale going forward.

I agree - this was one of the first things I asked about when we first released Pipes. Maybe we should just have a top-level 'Experimental' section where we put all experimental things until we decide if they're a go or not. It could be:

  • Easier for folks to find experimental stuff and try it out
  • Easier for us to add / maintain, since we don't have to think too hard about where the content will initially live

This approach can work for most things, but I'd like to have criteria for what gets defined as a core concept versus a product feature. I'm not saying Pipes shouldn't be that, but we have so many concepts and I want us to be selective (and consistent) about what we choose to include.

@yuhan yuhan merged commit 37d874b into master May 7, 2024
2 checks passed
@yuhan yuhan deleted the 03-25-docs_move_pipes_to_concepts branch May 7, 2024 17:46
@yuhan
Copy link
Contributor Author

yuhan commented May 7, 2024

Thanks Erin!

I'd like to have criteria for what gets defined as a core concept versus a product feature

We're going to do an API audit in the core concepts soon. That can be a good opportunity for us to come up with concept vs product guidelines.

sbquinlan pushed a commit that referenced this pull request May 7, 2024
## Summary & Motivation
this PR moves pipes pages to Concepts
- updates all internal links
- updates nav -- open to suggestions to the exact order of the nav
- sets up redirects so /guides/dagster-pipes URLs out there would still
work

## How I Tested These Changes
preview:
https://03-25-docs-move-pipes-to-concepts.dagster.dagster-docs.io/concepts/dagster-pipes

<img width="327" alt="image"
src="https://github.com/dagster-io/dagster/assets/4531914/0090db02-0d2c-43ac-8f91-2728c91430e2">
danielgafni pushed a commit to danielgafni/dagster that referenced this pull request Jun 18, 2024
## Summary & Motivation
this PR moves pipes pages to Concepts
- updates all internal links
- updates nav -- open to suggestions to the exact order of the nav
- sets up redirects so /guides/dagster-pipes URLs out there would still
work

## How I Tested These Changes
preview:
https://03-25-docs-move-pipes-to-concepts.dagster.dagster-docs.io/concepts/dagster-pipes

<img width="327" alt="image"
src="https://github.com/dagster-io/dagster/assets/4531914/0090db02-0d2c-43ac-8f91-2728c91430e2">
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