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] Add guide for code references #21755

Merged
merged 2 commits into from
Jul 8, 2024
Merged

Conversation

benpankow
Copy link
Member

@benpankow benpankow commented May 9, 2024

Summary

Add guide for basic local code references functionality. Will stack PR for source control references, when ready.

Test Plan

viewed locally

https://benpankow-guide-code-references.dagster.dagster-docs.io/guides/dagster/code-references

@graphite-app graphite-app bot requested a review from erinkcochran87 May 9, 2024 17:35
@graphite-app graphite-app bot added the area: docs Related to documentation in general label May 9, 2024
@benpankow
Copy link
Member Author

benpankow commented May 9, 2024

@benpankow benpankow requested review from sryza and rexledesma May 9, 2024 17:37
@@ -1159,13 +1159,13 @@
{
"title": "Experimental features",
"children": [
{
Copy link
Member Author

Choose a reason for hiding this comment

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

deprecated, so dropping

Copy link
Contributor

Choose a reason for hiding this comment

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

not a huge deal, but ideally would do this in a separate PR where we also drop the mdx and python files

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.

Looking good! One thing I'm not sure of is if it should be Code References or code references - is this a proprietary feature?

docs/content/guides/dagster/code-references.mdx Outdated Show resolved Hide resolved
docs/content/guides/dagster/code-references.mdx Outdated Show resolved Hide resolved
docs/content/guides/dagster/code-references.mdx Outdated Show resolved Hide resolved
docs/content/guides/dagster/code-references.mdx Outdated Show resolved Hide resolved
docs/content/guides/dagster/code-references.mdx Outdated Show resolved Hide resolved
docs/content/guides/dagster/code-references.mdx Outdated Show resolved Hide resolved
docs/content/guides/dagster/code-references.mdx Outdated Show resolved Hide resolved
@benpankow benpankow force-pushed the benpankow/guide-code-references branch from 7277f74 to 7ea5960 Compare May 15, 2024 18:35
@benpankow benpankow requested a review from erinkcochran87 May 15, 2024 18:44
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.

I'll leave it to the other reviewers to stamp for accuracy, but this looks good to me! I left a few small comments, pls address before merging 🙏

docs/content/guides/dagster/code-references.mdx Outdated Show resolved Hide resolved
docs/content/guides/dagster/code-references.mdx Outdated Show resolved Hide resolved
docs/content/guides/dagster/code-references.mdx Outdated Show resolved Hide resolved
assets=with_source_code_references(
[
my_asset,
another_asset,
Copy link
Contributor

@sryza sryza May 15, 2024

Choose a reason for hiding this comment

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

taking away the comma at the end of this line would allow this snippet to condense more

@@ -1159,13 +1159,13 @@
{
"title": "Experimental features",
"children": [
{
Copy link
Contributor

Choose a reason for hiding this comment

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

not a huge deal, but ideally would do this in a separate PR where we also drop the mdx and python files

docs/content/guides/dagster/code-references.mdx Outdated Show resolved Hide resolved
docs/content/guides/dagster/code-references.mdx Outdated Show resolved Hide resolved
docs/content/guides/dagster/code-references.mdx Outdated Show resolved Hide resolved
docs/content/guides/dagster/code-references.mdx Outdated Show resolved Hide resolved
file_path=os.path.join(
os.path.dirname(__file__), "source.yaml"
),
line_number=1,
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes me think we should make line_number optional

@benpankow benpankow force-pushed the benpankow/guide-code-references branch from 48eb1b8 to 62219d7 Compare May 16, 2024 21:16
@benpankow benpankow requested review from erinkcochran87 and sryza May 22, 2024 16:12
@benpankow benpankow force-pushed the benpankow/guide-code-references branch from 62219d7 to 739c3fa Compare May 22, 2024 16:12
@benpankow benpankow force-pushed the benpankow/guide-code-references branch from 739c3fa to 82dd9b7 Compare June 6, 2024 15:04
@benpankow benpankow force-pushed the benpankow/guide-code-references branch 2 times, most recently from b600ef7 to f3423f1 Compare June 7, 2024 22:06
Copy link
Contributor

@rexledesma rexledesma left a comment

Choose a reason for hiding this comment

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

Just clearing my queue -- let me know when this should get a final pass

@benpankow
Copy link
Member Author

benpankow commented Jun 11, 2024

This is ready for review on content, will need to update the screenshots since the design has changed a little

@benpankow benpankow requested a review from rexledesma June 11, 2024 16:01
@benpankow benpankow force-pushed the benpankow/guide-code-references branch from f3423f1 to 37567b6 Compare June 11, 2024 16:03
@benpankow benpankow force-pushed the benpankow/guide-code-references branch 3 times, most recently from d833a1c to b8f570e Compare July 3, 2024 20:46
@benpankow benpankow force-pushed the benpankow/guide-code-references branch from b8f570e to 9ff3f56 Compare July 8, 2024 20:33
@benpankow
Copy link
Member Author

benpankow commented Jul 8, 2024

Merge activity

  • Jul 8, 1:45 PM PDT: @benpankow started a stack merge that includes this pull request via Graphite.
  • Jul 8, 1:46 PM PDT: Graphite rebased this pull request as part of a merge.
  • Jul 8, 1:47 PM PDT: @benpankow merged this pull request with Graphite.

@benpankow benpankow force-pushed the benpankow/guide-code-references branch from 9ff3f56 to c8a8df7 Compare July 8, 2024 20:46
@benpankow benpankow merged commit 25b79c5 into master Jul 8, 2024
1 of 2 checks passed
@benpankow benpankow deleted the benpankow/guide-code-references branch July 8, 2024 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: docs Related to documentation in general
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants