etr2460 commented on a change in pull request #16454:
URL: https://github.com/apache/superset/pull/16454#discussion_r699740203



##########
File path: superset-frontend/src/datasource/DatasourceModal.tsx
##########
@@ -64,17 +64,17 @@ interface DatasourceModalProps {
   show: boolean;
 }
 
-function buildMetricExtraJsonObject(metric: Record<string, unknown>) {
+function buildExtraJsonObjects(item: Record<string, unknown>) {

Review comment:
       cool refactor, maybe name `buildExtraJsonObject` though since it only 
returns 1 object?

##########
File path: UPDATING.md
##########
@@ -25,31 +25,37 @@ assists people when migrating to a new version.
 ## Next
 
 ### Breaking Changes
+

Review comment:
       looks like we got some autogenerated changes here. maybe we should move 
those into another pr (if we think they're desirable?) 

##########
File path: superset/connectors/sqla/models.py
##########
@@ -267,6 +269,28 @@ def get_sqla_col(self, label: Optional[str] = None) -> 
Column:
     def datasource(self) -> RelationshipProperty:
         return self.table
 
+    def get_extra_dict(self) -> Dict[str, Any]:

Review comment:
       this is copy pasted from the metric model right? Maybe we can have a 
`CertifiableMixin` that both models can use?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to