geido commented on PR #21094:
URL: https://github.com/apache/superset/pull/21094#issuecomment-1222251745

   Hi @cccs-RyanK. Thanks for this! Before I dive into the code I have a 
general comment.
   I am not very fond of the idea of having constants, types and common 
components all within a common file. If we are opting for separation then I 
believe it makes sense to separate things a bit more.
   
   I'd prefer to see a structure similar to this one:
   
   - types.ts: Containing all the shared types
   - utils.ts: Containing shared functions and constants
   - BaseSelect.tsx: containing the shared component
   - This is optional, but we could also have a styles.ts for the common styled 
components
   
   @michael-s-molina what's your thought about this?


-- 
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