You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/07/06 09:17:06 UTC
[GitHub] [iceberg] XN137 opened a new pull request, #5210: Build: Upgrade test dependencies to latest version
XN137 opened a new pull request, #5210:
URL: https://github.com/apache/iceberg/pull/5210
- upgrade junit5 to 5.8.2
- upgrade assertj to 3.23.1
- upgrade mockserver to 5.13.2
- upgrade mockito to 4.6.1
- org.mockito.Matchers class has been removed
- refactor: prefer org.mockito.Mockito over ArgumentMatchers for consistency
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] XN137 commented on pull request #5210: Build: Upgrade test dependencies to latest version
Posted by GitBox <gi...@apache.org>.
XN137 commented on PR #5210:
URL: https://github.com/apache/iceberg/pull/5210#issuecomment-1176387280
rebased due to merge conflict
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on pull request #5210: Build: Upgrade test dependencies to latest version
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on PR #5210:
URL: https://github.com/apache/iceberg/pull/5210#issuecomment-1179418280
Yea , I do understand the desire to make it the same, but there's definitely many correct ways unless there is some definite rule, so I'd say let's revert those changes to focus this pr? Other changes look good to me.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on pull request #5210: Build: Upgrade test dependencies to latest version
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on PR #5210:
URL: https://github.com/apache/iceberg/pull/5210#issuecomment-1182780872
Merged, thanks @XN137 for change and @nastra for review
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] XN137 commented on pull request #5210: Build: Upgrade test dependencies to latest version
Posted by GitBox <gi...@apache.org>.
XN137 commented on PR #5210:
URL: https://github.com/apache/iceberg/pull/5210#issuecomment-1179501986
@szehon-ho ok i have removed the refactoring around `ArgumentMatchers` now
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on pull request #5210: Build: Upgrade test dependencies to latest version
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on PR #5210:
URL: https://github.com/apache/iceberg/pull/5210#issuecomment-1182152089
Missed the message, thanks, looks good to me. Will wait a little bit to see any other comments, otherwise will merge soon.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho merged pull request #5210: Build: Upgrade test dependencies to latest version
Posted by GitBox <gi...@apache.org>.
szehon-ho merged PR #5210:
URL: https://github.com/apache/iceberg/pull/5210
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on pull request #5210: Build: Upgrade test dependencies to latest version
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on PR #5210:
URL: https://github.com/apache/iceberg/pull/5210#issuecomment-1178365408
It makes sense to upgrade to me, but do you think we can leave out the ArgumentMatchers => Mockito refactor? It doesn't seem that valuable unless its enforced going forward, one way or the other. As any() is defined on ArgumentMatchers, I was actually thinking it makes sense to call it from where it is defined.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] XN137 commented on pull request #5210: Build: Upgrade test dependencies to latest version
Posted by GitBox <gi...@apache.org>.
XN137 commented on PR #5210:
URL: https://github.com/apache/iceberg/pull/5210#issuecomment-1178639093
> As any() is defined on ArgumentMatchers, I was actually thinking it makes sense to call it from where it is defined.
this was the situation on master:
```
~/code/iceberg$ rg "Mockito.any" | wc -l
45
~/code/iceberg$ rg "ArgumentMatchers.any" | wc -l
9
```
so using the any-methods from Mockito seems to be preferred across the codebase.
thus i replaced the now deleted `Matchers` with `Mockito`
and refactored `ArgumentMatchers` usage as well for consistency.
i think generally it is not unusual to use wildcard imports with Mockito as an exception:
`import static org.mockito.Mockito.*;`
or just import `Mockito` and have all related methods discoverable from a single entry point (i.e. auto-complete).
that being said, i am fine to revert this part of the change if you still want me to?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org