gaborkaszab commented on PR #14347: URL: https://github.com/apache/iceberg/pull/14347#issuecomment-3411616940
> Thank you for change @gaborkaszab > > I am bit confused on why do we want to remove tests when config is still there, wouldn't it be better to first let the property be removed ( in 2.0.0) then either with it or in seperate pr remove the test ? Thanks for taking a look, @singhpk234 ! These tests actually don't test anything, because MANIFEST_LISTS_ENABLED is not used anywhere now. So there are 3 tests currently (testWriteNewManifestsIdempotency, and the 2 that I dropped) that does exactly the same, with the only difference that 2 of them adjusts a setting that doesn't do anything. My other motivation is that on the community sync it came up that we might want to reduce the lifecycle of deprecated functionality in the core module from 2.0.0, because that module isn't that strict as the API module. So I figured that I check of usages of such deprecated members/functions, and in case there is an alternative path to use I'll remove the usage of the deprecated ones so that we can easily remove them in a later release. In this process I found that this MANIFEST_LIST_ENABLED only has usage in test but not in prod code. Hope this makes sense! -- 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]
