You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by "nandakumar131 (via GitHub)" <gi...@apache.org> on 2023/12/13 11:49:45 UTC

[PR] RATIS-1970. Add junit 5 dependencies. [ratis]

nandakumar131 opened a new pull request, #993:
URL: https://github.com/apache/ratis/pull/993

   ## What changes were proposed in this pull request?
   
   Junit 5 dependencies are added to the appropriate pom files.
   
   ## What is the link to the Apache JIRA
   
   RATIS-1970
   
   ## How was this patch tested?
   
   Existing unit tests.
   


-- 
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@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] RATIS-1970. Add junit 5 dependencies. [ratis]

Posted by "nandakumar131 (via GitHub)" <gi...@apache.org>.
nandakumar131 commented on PR #993:
URL: https://github.com/apache/ratis/pull/993#issuecomment-1855114073

   Thanks for the review @adoroszlai &  @szetszwo.
   
   @szetszwo, sure. will do.
   
   FYI: The scope of the junit dependencies are of scope `test`, so it will not be part of Ratis's transitive dependencies.


-- 
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@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] RATIS-1970. Add junit 5 dependencies. [ratis]

Posted by "nandakumar131 (via GitHub)" <gi...@apache.org>.
nandakumar131 commented on PR #993:
URL: https://github.com/apache/ratis/pull/993#issuecomment-1855240130

   @szetszwo, I have been working on limiting the changes just to `ratis-common`, but the test compilation started failing on `ratis-server`, `ratis-test` & `ratis-examples` as they extend `BaseTest` from `ratis-common`.
   
   I will try to limit the changes on each PR to make the review process easier.
   
   This PR will only update the parent `pom.xml` and `ratis-common`'s `pom.xml`
   
   Test-case changes will happen in follow-up PRs.


-- 
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@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] RATIS-1970. Add junit 5 dependencies. [ratis]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #993:
URL: https://github.com/apache/ratis/pull/993#issuecomment-1854576933

   @nandakumar131 , thanks a lot for working on JUnit5 migration!  For the dependencies, let's add them when they are needed (i.e. add them with the tests).  We minimize the dependencies because Ratis is a library.  We should make it easier to be consumed.
   
   For JUnit5 migration, we may start with ratis-common since it only has a few tests.  It can show how the migration works.


-- 
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@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] RATIS-1970. Add junit 5 dependencies. [ratis]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo merged PR #993:
URL: https://github.com/apache/ratis/pull/993


-- 
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@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] RATIS-1970. Add junit 5 dependencies. [ratis]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #993:
URL: https://github.com/apache/ratis/pull/993#issuecomment-1857075903

   > FYI: The scope of the junit dependencies are of scope test, so it will not be part of Ratis's transitive dependencies.
   
   @nandakumar131 , I understand that the test dependencies are not transitive.  That's a good point!
   
   It is usually hard to remove dependencies.  So I really oppose adding unused dependencies.  Moreover, we have to handle it in case of CVEs.
   
   Please include some change to use the dependencies in this pull requests.  


-- 
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@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org