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

Ct 1518/convert 063 relation names tests #6304

Merged
merged 4 commits into from
Nov 29, 2022

Conversation

VersusFacit
Copy link
Contributor

resolves #6274

Description

Conversion of a test, but I took the time to shape them up for modern standards and rewrite code/document it in a way (I hope) that makes their intent more readable.

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have signed the CLA
  • 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
  • I have opened an issue to add/update docs, or docs changes are not required/relevant for this PR

@VersusFacit VersusFacit added the Skip Changelog Skips GHA to check for changelog file label Nov 22, 2022
@VersusFacit VersusFacit requested a review from a team as a code owner November 22, 2022 05:46
@cla-bot cla-bot bot added the cla:yes label Nov 22, 2022
Comment on lines +78 to +81
# Backup table name generation:
# 1. for len(relation name) <= 51, backfills
# 2. for len(relation name) > 51 characters, overwrites
# the last 12 characters with __dbt_backup
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: does this still need to be here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely imo. It's the comments that explain what's going on with the backup name truncation rules!

Copy link
Contributor

@McKnight-42 McKnight-42 left a comment

Choose a reason for hiding this comment

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

This all look good to me other than my one question about some comments.

@VersusFacit VersusFacit merged commit 47c2edb into main Nov 29, 2022
@VersusFacit VersusFacit deleted the CT-1518/convert_063_relation_names_tests branch November 29, 2022 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes Skip Changelog Skips GHA to check for changelog file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-1518] 063_relation_name_tests
2 participants