nandorKollar commented on PR #431: URL: https://github.com/apache/iceberg-go/pull/431#issuecomment-2958305165
> Thanks a lot @nandorKollar @zeroshade @laskoviymishka for your review!! really appreciate it! I have tried to address majority of them, a couple of them are remaining > > summary is > > * no panics returning error now > * moved updateschema from table to transaction > * changes delete, add , updates to map of int (ids) > * simplified addcolumn logic > * using 1 function for updates (@zeroshade your review here if this is what you were looking for ) > * added tests > > @zeroshade in add column or in general while accepting column name should move from `[]string` to just `string` ? and then split ` parts := strings.Split(name, ".") ` > > wouldnt this be easier ? > > also need more a little more help in understanding how do we handle the reference of txn here ? Thanks, I think from readability point of view, this looks much better. I need some more time to go through the details. -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
