-
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
Add Conversion Metrics Properties #209
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. |
549313e
to
8c2f986
Compare
8c2f986
to
30890ae
Compare
30890ae
to
b93ac79
Compare
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.
I think there's a general issue of too many accessors, and I think we'll be better off starting with more streamlined protocols. In particular, I'd like to avoid the get_property_or_error
accessor pattern you've introduced here.
Otherwise, this looks good. We can see if @QMalcolm has any added feedback while we clean up the accessors.
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.
This is looking really good! Nothing major from me. @tlento has raised some really good points which I'd also like to see addressed 🙂
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! Thank you for doing this work 🙂
### Description Backporting conversion metric support PRs to `0.4.latest` - #221 - #209 <!--- Describe the Pull Request here. Add any references and info to help reviewers understand your changes. Include any tradeoffs you considered. --> ### Checklist - [x] I have read [the contributing guide](https://github.com/dbt-labs/dbt-semantic-interfaces/blob/main/CONTRIBUTING.md) and understand what's expected of me - [x] I have signed the [CLA](https://docs.getdbt.com/docs/contributor-license-agreements) - [x] This PR includes tests, or tests are not required/relevant for this PR - [x] I have run `changie new` to [create a changelog entry](https://github.com/dbt-labs/dbt-semantic-interfaces/blob/main/CONTRIBUTING.md#adding-a-changelog-entry)
Description
Spec
Reference #210
Checklist
changie new
to create a changelog entry