-
Notifications
You must be signed in to change notification settings - Fork 14
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
Rename SavedQuery.group_bys
to SavedQuery.group_by
#186
Conversation
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
9d011fe
to
d2cb710
Compare
@QMalcolm - How do we go about handling this kind of rename on the |
We can't, at least not for dbt-core 1.7, and that is a for a few reasons
The only way to do this currently for dbt-core 1.7.0 and DSI 0.3.0 would be to support both, which also seems undesireable |
Also I have a few separate questions:
|
We'll put this into 0.4.0 and then jam that into the final cut of dbt-core 1.7.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
group_bys
field in SavedQueries to group_by
SavedQuery.group_bys
to SavedQuery.group_by
d2cb710
to
2678fea
Compare
@QMalcolm - Good point, created an issue in DSI and updated links. |
In dbt-labs/dbt-semantic-interfaces#186 we renamed `group_bys` to `group_by` in DSI (which went out in DSI 0.4.0). The original `group_bys` naming was a typo. This fixes that. This is a non-breaking change from the core perspective because saved queries are being introduced in 1.7.0, which has not been cut yet.
In dbt-labs/dbt-semantic-interfaces#186 we renamed `group_bys` to `group_by` in DSI (which went out in DSI 0.4.0). The original `group_bys` naming was a typo. This fixes that. This is a non-breaking change from the core perspective because saved queries are being introduced in 1.7.0, which has not been cut yet.
Resolves #192
Description
Please see associated issue for more details.
Checklist
changie new
to create a changelog entry