kaspersorensen commented on issue #230: dev/add datatypes to excel columns URL: https://github.com/apache/metamodel/pull/230#issuecomment-535814659 It would have been nice to have a small discussion about this change before doing the PR because I feel like there's maybe a few things I would have liked to do differently: * The idea of detecting column type seems general enough that we should have a facility for it as a decorator pattern or such. * The PR does not do anything to ensure conversion of values to conform with the data types. I know we have a converter API for that in the core module. * I do like the change from VARCHAR to STRING. For the sake of separating concerns I would do that in a separate PR though.
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
