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

Simplify the PR checklist in relation to testing #906

Closed
dbeatty10 opened this issue Jun 4, 2024 · 0 comments · Fixed by #907
Closed

Simplify the PR checklist in relation to testing #906

dbeatty10 opened this issue Jun 4, 2024 · 0 comments · Fixed by #907

Comments

@dbeatty10
Copy link
Contributor

dbeatty10 commented Jun 4, 2024

Describe the "under the hood" change

Our PR template is pretty long, which makes it onerous to use. It is also unnecessarily different from the PR templates of dbt-core, dbt-adapters, etc.

So we should replace these (6) checklist items:

  • I have verified that these changes work locally on the following warehouses (Note: it's okay if you do not have access to all warehouses, this helps us understand what has been covered)
    • BigQuery
    • Postgres
    • Redshift
    • Snowflake
  • I have added tests & descriptions to my models (and macros if applicable)

With these (2) checklist items:

  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR

Any bug fixes and feature requests should have associated tests, and those tests should run against all the aforementioned data platforms.

Who will this benefit?

This will reduce the length of the PR checklist, which will make it easier to complete (in alignment with the suggestions within the Checklist Manifesto).

This will also more closely align the PR template within dbt-utils with the PR templates of dbt-core, dbt-adapters, etc.

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 a pull request may close this issue.

1 participant