nastra commented on PR #190: URL: https://github.com/apache/parquet-site/pull/190#issuecomment-4932859582
> FWIW I don't understand why team junit set out to break every single assert(), especially how assertEquals(message, expected, actual) still compiles but now the args become something else like assert(actual, expected, message). > my guess is because they realized that it's more natural to mentally define `actual` and then `expected`. I've also seen numerous cases across the parquet-java codebase where the actual/expected were flipped, which is easy to do JUnit assertions. With AssertJ assertions you're always defining the `actual` first and then verify that certain things hold on that object > assertJ just takes all the pain away. > > Does break all pending PRs with tests though, such as my own #3562; forced an update and a rebase there... This is true unfortunately that this will cause conflicts on pending PRs and I don't have a good solution on how to avoid that. In Iceberg we migrated module by module and had to deal with merge conflicts as well for a while, but the Iceberg codebase is larger (especially multiple versions of Flink + Spark modules). For parquet-java the migration should be much easier/quicker, so we could technically migrate the codebase to AssertJ in a short amount of time to reduce future conflicts on newer PRs. I already have most of the work done but haven't published all PRs yet, because I don't want to overwhelm reviewers. -- 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]
