[
https://issues.apache.org/jira/browse/YARN-8943?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16804298#comment-16804298
]
Szilard Nemeth commented on YARN-8943:
--------------------------------------
Hi [~ajisakaa]!
I reviewed your changes, +1 (non-binding).
Next time, please avoid to change things not strictly related to the patch,
e.g.:
* Changing method visibilities
* Reformatting the code
* Any other unnecessary change
These can be solved in a follow-up jira, but in this case, just makes the
review process harder.
This is especially true if you introduce junit5 into some bigger Maven modules
than the hadoop-yarn-api project (I saw you have this in YARN-6946).
I do note that the order of the parameters needs to be changed for the assertXX
methods, as the message string is became the last parameter in junit5 as
opposed to junit4.
Btw, after this is merged, are you planning to proceed with YARN-6946?
As I said, if you include the changes absolutely required to introduce junit5
into that project, the more likely it's easier to review.
A second thought, just out of curiosity: Did you use some junit4 to junit5
migration script or did you need to adjust the order of the parameters by hand?
Thanks!
> Upgrade JUnit from 4 to 5 in hadoop-yarn-api
> --------------------------------------------
>
> Key: YARN-8943
> URL: https://issues.apache.org/jira/browse/YARN-8943
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: test
> Reporter: Akira Ajisaka
> Assignee: Akira Ajisaka
> Priority: Major
> Attachments: YARN-8943.01.patch, YARN-8943.02.patch
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
