[GitHub] williaster commented on issue #5671: Refactor Chord vis

[GitBox <gi...@apache.org>]

williaster commented on issue #5671: Refactor Chord vis
URL: https://github.com/apache/incubator-superset/pull/5671#issuecomment-414136464
 
 
   wohoo! 🎉 this is the start of an amazing thing! 👯📈
   
   overall looks great, a couple questions/thoughts: 
   
   - is it worth starting to define consistent data shapes with this decoupling? It might be too early for that but it seems potentially worthwhile to define/track the expected data shape (since you have the context of the vis fresh while doing this). we could define the data shape as a `PropTypes.shape({ .. })` and then comment them out in the file until later.
   
   - do you think we should move adapters to a separate file or directory? I'm a fan of small files for easy readability but again, it might be too early to start doing that/could just do in one PR later 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org