[GitHub] [iceberg] rustyconover commented on pull request #8067: Python: Add more tests for the Avro writer

["rustyconover (via GitHub)" <gi...@apache.org>]

rustyconover commented on PR #8067:
URL: https://github.com/apache/iceberg/pull/8067#issuecomment-1674146191

   Hi Friends,
   
   Tonight I'm trying to rebase my Avro reader branch and this PR really wasn't easy to follow. 
   
   The PR change description didn't really describe what was decided to be changed without reading all of the comments from the review.
   
   In the future can you please split things like this up with clearer logic written in the change message especially decisions around architecture (such as don't use native date/datetime types).  It took me a while to understand why tests were removed.
   
   I'd like to see the commits/PRs say things like:
   
   "Removing use of native datetime types in the Avro reader path, removing relevant tests."
   
   Talking about fixing bugs in the read path just didn't give the justifications why the changes were made.
   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org