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

remove callout and add tabs as examples #4402

Merged
merged 14 commits into from
Nov 8, 2023
Merged

remove callout and add tabs as examples #4402

merged 14 commits into from
Nov 8, 2023

Conversation

mirnawong1
Copy link
Contributor

addressing @joellabes' request to remove old callouts for alias, database, and schema pages. also added example tabs at the beginning to help provide users with more example at the beginning of the doc

@mirnawong1 mirnawong1 requested a review from a team as a code owner November 6, 2023 11:23
@vercel Vercel
Copy link

vercel bot commented Nov 6, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs-getdbt-com ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 8, 2023 9:39pm

@github-actions github-actions bot added size: medium This change will take up to a week to address content Improvements or additions to content Docs team Authored by the Docs team @dbt Labs labels Nov 6, 2023
joellabes
joellabes requested changes Nov 7, 2023
View reviewed changes
Copy link
Contributor

@joellabes joellabes left a comment

Choose a reason for hiding this comment

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

ty @mirnawong1! Added a few comments and tweaks but this is going to be so much better 😍

website/docs/reference/resource-configs/alias.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/alias.md Show resolved Hide resolved
website/docs/reference/resource-configs/alias.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/alias.md Show resolved Hide resolved
website/docs/reference/resource-configs/alias.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/alias.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/database.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/database.md Show resolved Hide resolved
website/docs/reference/resource-configs/schema.md Show resolved Hide resolved
website/docs/reference/resource-configs/schema.md Show resolved Hide resolved
mirnawong1
mirnawong1 commented Nov 7, 2023
View reviewed changes
website/docs/reference/resource-configs/alias.md
</File>

This would return `analytics.finance.sales_dashboard` in the database, instead of the default `analytics.finance.sales_metrics`.
</TabItem>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

or is it analytics.finance.unique_order_id_test? this makes me think it's the _ one?

mirnawong1
mirnawong1 commented Nov 7, 2023
View reviewed changes
website/docs/reference/resource-configs/alias.md
</File>

This would return `analytics.finance.sales_dashboard` in the database, instead of the default `analytics.finance.sales_metrics`.
</TabItem>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

do we add config or is that only for properties.yml?

@mirnawong1
Copy link
Contributor Author

hey @joellabes thank you so much for this! I've made changes to it and added items you suggested. i did ask about tests in custom database to confirm, but i also added an example just in case. also i have a question below for you if you can clarify because i see some do and some don't based on the issues you linked above. thank you!

  • can you also use config block for all 4 resource types?

@github-actions github-actions bot added size: large This change will more than a week to address and might require more than one person and removed size: medium This change will take up to a week to address labels Nov 7, 2023
joellabes
joellabes requested changes Nov 8, 2023
View reviewed changes
Copy link
Contributor

@joellabes joellabes left a comment

Choose a reason for hiding this comment

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

Looking better! A couple of tweaks on the net-new stuff again

website/docs/reference/resource-configs/alias.md Outdated

<TabItem value="test" label="Tests">

Configure a test's alias in your `dbt_project.yml` file.
Copy link
Contributor

Choose a reason for hiding this comment

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

You can't create tests in a dbt_project.yml, this would be in a schema.yml file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you! wait is schema.yml the same as properties.yml? https://docs.getdbt.com/reference/configs-and-properties#schemayml-files

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep!

website/docs/reference/resource-configs/alias.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/alias.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/alias.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/database.md Outdated
+database: staging
```

This would result in the generated relation being located in the `staging` database, so the full relation name would be `staging.finance.product_categories_data`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Where did the _data suffix come from here?

Copy link
Contributor Author

@mirnawong1 mirnawong1 Nov 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

oh fudge! i think i copied the over from alias and didnt remove the _data bit

@joellabes
Copy link
Contributor

  • can you also use config block for all 4 resource types?

Yes:

models:
  - name: first_model
    config:
      alias: second_model
    columns:
      - name: id
        tests:
          - unique:
              config:
                alias: custom_test_name_when_storing_failures

snapshots:
  - name: first_snapshot
    config:
      alias: second_snapshot

mirnawong1
mirnawong1 commented Nov 8, 2023
View reviewed changes
Copy link
Contributor Author

@mirnawong1 mirnawong1 left a comment

Choose a reason for hiding this comment

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

thank you @joellabes ! ok added your feedback now - can you re-review when you have a sec?

@mirnawong1
Copy link
Contributor Author

going to enable automerge in case it's all good to go

@mirnawong1 mirnawong1 enabled auto-merge November 8, 2023 13:04
@mirnawong1 mirnawong1 merged commit aebba3c into current Nov 8, 2023
4 checks passed
@mirnawong1 mirnawong1 deleted the mwong-remove-callout branch November 8, 2023 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joellabes joellabes joellabes approved these changes

Assignees
No one assigned
Labels
content Improvements or additions to content Docs team Authored by the Docs team @dbt Labs size: large This change will more than a week to address and might require more than one person
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mirnawong1 @joellabes