-
Notifications
You must be signed in to change notification settings - Fork 1
CLL: Add impact radius of a column #17
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
CLL: Add impact radius of a column #17
Conversation
Signed-off-by: popcorny <celu@infuseai.io>
DaveFlynn
left a comment
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.
Just one note about adding an explanation to the example section. Other than that, it's looks good.
|
|
||
| ### Example: Simplified Model Chain | ||
| {: .shadow} | ||
|
|
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.
Some textual explanation of the image would help, in addition to the raw SQL from the models.
To help the reader understand how CLL can help them to visualize the relations. Of course, they can figure it out from the SQL by themself, but we can speed that up with a brief, plain language, description
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.
ijac13
left a comment
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.
LGTM.
We may need to have a separate doc for impact radius
Signed-off-by: popcorny <celu@infuseai.io>
DaveFlynn
left a comment
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.
Two sentence adjustments, other than that it's good to go!
Signed-off-by: popcorny <celu@infuseai.io>
DaveFlynn
left a comment
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.
LGTM

We add the model-to-column dependencies, so we now can show the impact radius of a column