[GitHub] [ratis] szetszwo commented on pull request #652: RATIS-1588. Add a builder of RaftStorageImpl

[GitBox <gi...@apache.org>]

szetszwo commented on PR #652:
URL: https://github.com/apache/ratis/pull/652#issuecomment-1155394043

   @codings-dan , The test failure seems related.  Could you take a look?
   
   BTW, let's remove the public RaftStorageImpl constructor below since it is no longer used.
   ```
     public RaftStorageImpl(File dir, CorruptionPolicy logCorruptionPolicy,
         long storageFeeSpaceMin) throws IOException {
   ```
   
   Also, let's use SizeInBytes for storageFreeSpaceMin? Or, we may have two setStorageFreeSpaceMin methods.
   ```
       public Builder setStorageFreeSpaceMin(SizeInBytes storageFreeSpaceMin) {
         this.storageFreeSpaceMin = storageFreeSpaceMin.getSize();
         return this;
       }
   
       public Builder setStorageFreeSpaceMin(long storageFreeSpaceMin) {
         this.storageFreeSpaceMin = storageFreeSpaceMin;
         return this;
       }
   ```


-- 
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