jorabin / KeePassJava2

Java API for KeePass Password Databases - Read/Write 2.x (File versions 3 and 4), Read 1.x
Apache License 2.0
250 stars 71 forks source link

Replace Google's guava library by Apache's commons-io. #59

Closed Siggen closed 2 months ago

Siggen commented 6 months ago

This PR remove guava library and replace it by commons-io.

Reasons are :

Thank in advance !

jorabin commented 2 months ago

Hi, Thank you for this pull request. I'm so sorry not to have spotted it sooner and hence not to have replied to it.

The LittleEndianDataInputSTream and OutputStream classes in Guava are both noted as @Beta and have been for a very long time, so in principle I'd prefer not to use them. However, I assume that they both have test cases and from that point of view would prefer to use them than to create a local class, as seems to be necessary, as it looks like there is no equivalent to LittleEndianDataOutputStream in Apache.

Removing any dependency is good, but this looks like a swap of guava for commons-io, which doesn't look as though it's used anywhere else in the project.

Given the need to add testing for LittleEndianOutputStream, which I don't see in this PR, I don't see compelling benefit in making the replacement, but perhaps I am missing the point?

thanks for the input

jorabin commented 2 months ago

I should have added that i'd welcome further discussion on this point, but otherwise won't take any further action.

Siggen commented 2 months ago

Hi Jo,

You're right, I'll try to add some testing for LittleEndianOutputStream to complete this PR. I don't have much spare bandwith at the moment, it might take some time to appear though.

The compelling benefit (for me) is getting rid of guava which I try to avoid including in my projects (it's big and duplicated a lot of functionalities found in other libraries). I understand your point of view may be different, no problem.

I also understand you would prefer not including commons-io, I'll see if I can do something about it.

I'll keep you informed.

Thank you !

jorabin commented 2 months ago

Thanks

Like I say, unless other people have a strong preference for swapping out Guava, I do not see a case for it

thanks Jo

jorabin commented 2 months ago

Given the release of 2.2.2 and #60 (2.2.3) and #62 (3.0.0) I think this PR at least needs major revision.

Thanks very much for the contribution, let's continue the discussion under #62 where we could also discuss adoption of Junit 5 and other improvements, including planned move to Java 11 baseline.