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

Feature/generate_source_descs_iss64 #66

Merged
merged 11 commits into from
Jun 22, 2022

Conversation

kbrock91
Copy link
Contributor

@kbrock91 kbrock91 commented Jun 10, 2022

This is a:

  • bug fix PR with no breaking changes — please ensure the base branch is main
  • new functionality — please ensure the base branch is the latest dev/ branch
  • a breaking change — please ensure the base branch is the latest dev/ branch

Description & motivation

Currently, generate_source has an option to set include_descriptions = True, but this parameters only includes descriptions at the column level. Ideally, description placeholders would also be generated for the source and tables as well. Additionally, the required parameter for generate_source macro is the schema name, but there is no option to input a name value. It is possible that a user would like to name their source a different name from the schema name. See issue

Checklist

  • I have verified that these changes work locally
  • I have updated the README.md (if applicable)
  • I have added tests & descriptions to my models (and macros if applicable)
  • I have added an entry to CHANGELOG.md

Copy link
Contributor

@b-per b-per left a comment

Choose a reason for hiding this comment

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

Thanks @kbrock91 . Looks Good To Me!

Copy link
Contributor

@matt-winkler matt-winkler left a comment

Choose a reason for hiding this comment

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

Same here - looks straightforward.

Copy link
Contributor

@dbeatty10 dbeatty10 left a comment

Choose a reason for hiding this comment

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

Awesome @kbrock91 💪

I left comments for a couple very small tweaks. If we can get those incorporated, then we can include this in the next release.

macros/generate_source.sql Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
kbrock91 and others added 3 commits June 22, 2022 12:32
Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com>
Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com>
kbrock91 and others added 2 commits June 22, 2022 12:44
Copy link
Contributor

@dbeatty10 dbeatty10 left a comment

Choose a reason for hiding this comment

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

Looks great 🎯

@dbeatty10 dbeatty10 merged commit 15c5ec3 into main Jun 22, 2022
@dbeatty10 dbeatty10 mentioned this pull request Jun 22, 2022
7 tasks
jeremyholtzman pushed a commit that referenced this pull request Apr 10, 2023
* update generate_source.sql to add descriptions for table/source and name arg

* add integration tests

* fix test

* update all args source test

* changelog

* Update macros/generate_source.sql

Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com>

* Update README.md

Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com>

* moved name to end up arguments block in readme

* Update integration_tests/tests/test_generate_source_all_args.sql

Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com>

* move name to last arg in test_generate_source_all_args

Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com>
@gwenwindflower gwenwindflower deleted the feature/generate_source_descs_iss64 branch February 28, 2024 23:01
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.

enhancements for generate_source to include source/table descriptions and source name
4 participants