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.

Sign up for GitHub

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

Jump to bottom

[docs] - Asset selection syntax updates #18041

Merged
merged 11 commits into from
Nov 16, 2023
Merged

[docs] - Asset selection syntax updates #18041

merged 11 commits into from
Nov 16, 2023

Conversation

erinkcochran87
Copy link
Contributor

@erinkcochran87 erinkcochran87 commented Nov 15, 2023 •
edited
Loading

Summary & Motivation

This PR cleans up the Asset selection guide and adds links to the guide elsewhere in the docs. Specifically:

  • Adds section headings to syntax guide
  • Puts the syntax/usage reference into a table
  • Adds an expanded Examples section. For each scenario, added examples in Python, the CLI, and the Dagster UI. I opted to do this because there seemed to be (at least during my testing) subtle differences in syntax for each approach. Also, visuals can help make it clear what to expect for different queries.

Before:

Screenshot 2023-11-16 at 10 45 12 AM

After:

Screenshot 2023-11-16 at 10 45 32 AM
Screenshot 2023-11-16 at 10 49 30 AM

How I Tested These Changes

👀 , DU project + CLI, UI, Python

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 15, 2023 •
edited
Loading

Deploy preview for dagster-docs ready!

Preview available at https://dagster-docs-fwb640kq5-elementl.vercel.app
https://erin-asset-selection-syntax.dagster.dagster-docs.io

Direct link to changed pages:

jamiedemaria
jamiedemaria reviewed Nov 16, 2023
View reviewed changes
Copy link
Contributor

@jamiedemaria jamiedemaria 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 really good. once the TODOs are complete, it looks gtg to me

docs/content/concepts/assets/asset-selection-syntax.mdx Show resolved Hide resolved
jamiedemaria
jamiedemaria approved these changes Nov 16, 2023
View reviewed changes
Copy link
Contributor

@jamiedemaria jamiedemaria left a comment

Choose a reason for hiding this comment

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

!!

@erinkcochran87 erinkcochran87 merged commit 30881ea into master Nov 16, 2023
1 check passed
@erinkcochran87 erinkcochran87 deleted the erin/asset-selection-syntax branch November 16, 2023 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamiedemaria jamiedemaria jamiedemaria approved these changes

@sryza sryza Awaiting requested review from sryza

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@erinkcochran87 @jamiedemaria