RussellSpitzer commented on PR #3256: URL: https://github.com/apache/polaris/pull/3256#issuecomment-3773649070
> @flyrain @RussellSpitzer I am working on the removal of the `object-storage-mock` and other test dependencies. I should be able to propose something today or tomorrow. > > Would it be possible to do that in a follow-up PR? Rationale: this one has already been increased by +1k LoC with the removal of Nessie code in the production scope. And I think the testing scope would be trivial to review in isolation. This is actually a good argument that the Nessie code should be in a separate PR which we would merge first :) > > In other words: do you consider this dependency to Nessie libs for testing purposes as a blocker? Honestly if we go by normal OSS project patterns, breaking this PR itself into multiple sub-pr's is probably more appropriate. I've seen in past PR's in the Polaris project we are tending to merge large blocks of code into the project then patch things up post facto. This isn't really a good pattern since it necessarily biases towards contributors who have enough insider connections to be "trusted" to follow up. I think it would be best in this case to get in these testing deps in in separate PR's (including the Iceberg table construction code) prior to merging this code. I haven't been as active in the project lately due to new children though, so I don't want to disrupt what's going on but my gut would say this PR is trying to bite off too much in one go. A good question is would accept this from a PR from a new contributor to the project? Say a contributor to Gravatino wrote this same PR but had used Gravitino as a test dependency. So i'm a -0, but mostly because I want to convince everyone else to follow more normal review procedures if possible. I'm just one community member 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
