slfan1989 commented on PR #6418: URL: https://github.com/apache/hadoop/pull/6418#issuecomment-1879646581
> > my personal opinion, if we use `AssertJ`, for one module (like hadoop-common), I think they should be done together, like in [HADOOP-19025](https://issues.apache.org/jira/browse/HADOOP-19025), instead of submitting multiple PRs. > > I agree, we should strive for manageable size patches. Huge patches are hard to review, tiny patches increase noise in PRs/commits/CI builds. > > I asked because even [HADOOP-19025](https://issues.apache.org/jira/browse/HADOOP-19025) does not cover the complete module (`hadoop-common-project/hadoop-common`), but I think at ~80k it's already big enough. > > On a related note, should we add static import for `assertThat`? I followed existing practice in [HADOOP-19025](https://issues.apache.org/jira/browse/HADOOP-19025) (importing `Assertions` and using `Assertions.assertThat`), but would prefer static import instead (the way JUnit assertions are used in the same files). HADOOP-19025 has been changed a lot, which makes sense to me. Let's continue the discussion with HADOOP-19025 and wait for Steve's suggestion, he has better ideas for unit test upgrades. -- 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]
