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

Feat/generate sources add database and schema #124

Merged
merged 6 commits into from
May 1, 2023

Conversation

jeremyholtzman
Copy link
Contributor

resolves #123

This is a:

  • documentation update
  • bug fix with no breaking changes
  • new functionality
  • a breaking change

Description & motivation

Checklist

  • This code is associated with an issue which has been triaged and accepted for development.
  • 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

@jeremyholtzman jeremyholtzman force-pushed the feat/generate_sources_add_database_and_schema branch 2 times, most recently from 6cecf1c to de916e9 Compare April 10, 2023 17:50
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.

Now that we're aligned on the behavior described in #123 (comment), we can tackle all the implementation details 🚀

Can you do each of the following before we dive any deeper into the code review?

  1. Remove any code formatting not related to the actual implementation of the new feature
  2. Use the following two (shorter) parameter names:
    • include_database
    • include_schema

More detail

There appear to be a lot of unnecessary code formatting, so I didn't go through and make code suggestions for each. You'll be able to tell if you caught them all if the diff shown within the pull request is minimal.

@jeremyholtzman
Copy link
Contributor Author

@dbeatty10 Thanks Doug, just made the updates you mentioned!

CHANGELOG.md Outdated Show resolved Hide resolved
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 @jeremyholtzman -- thanks for all your work on this!

@dbeatty10 dbeatty10 merged commit 2012ee8 into main May 1, 2023
@gwenwindflower gwenwindflower deleted the feat/generate_sources_add_database_and_schema 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.

Add optional arguments to include database and schema properties in sources.yml generated from generate_source
2 participants