commons-beanutils icon indicating copy to clipboard operation
commons-beanutils copied to clipboard

Add Converters for Additional Types

Open SethFalco opened this issue 4 years ago • 6 comments

This adds the following converters, with test cases:

  • Color
  • Dimension
  • InetAddress
  • Locale
  • Pattern
  • Point

You can see the test cases for example inputs/outputs.

I've just taken my PR from DeltaSpike (https://github.com/apache/deltaspike/pull/109) and converted the converters that didn't exist here to work with this project instead. This is just me acting on a comment in the PR (https://github.com/apache/deltaspike/pull/109#issuecomment-720469005) in case it does happen, I figured I could have the PR here already.

This does not port all changes from my other PR, for example I implemented some existing converters differently, but I'll PR those separately, so they can just be closed if they're unfavorable.

SethFalco avatar Nov 03 '20 14:11 SethFalco

Definitely submit your PR of your other changes. I would like to see it and I did some of these converters and wasn't quite sure the best way to handle so i will be interested to see what you have done.

melloware avatar Nov 03 '20 14:11 melloware

Also rebase this one please.

melloware avatar Dec 30 '20 02:12 melloware

Thanks for the comments, I believe both should be resolved now. Let me know if there're any other issues.

SethFalco avatar Dec 30 '20 16:12 SethFalco

Sorry for the delay in coming back to this. Ended up getting pretty busy, then put this off for a while.

  • I've rebased with master.
  • Squashed my commits.
  • I believe I've resolved all of your feedback.

High hopes everything is cool, but I'll be available if you have any further feedback.

SethFalco avatar Jul 09 '21 03:07 SethFalco

@garydgregory If this PR it too large to review at once, would you prefer if I split it up into smaller ones?

I could do one PR per converter for example?

SethFalco avatar Apr 25 '22 11:04 SethFalco

I'm sure it's fine for now (13 files).

Gary

On Mon, Apr 25, 2022, 07:28 Seth Falco @.***> wrote:

@garydgregory https://github.com/garydgregory If this PR it too large to review at once, would you prefer if I split it up into smaller ones?

I could do one PR per converter for example?

— Reply to this email directly, view it on GitHub https://github.com/apache/commons-beanutils/pull/47#issuecomment-1108449860, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJB6N6CVEQW2JMEWHTDT6LVGZ6VDANCNFSM4TIZJIZA . You are receiving this because you were mentioned.Message ID: @.***>

garydgregory avatar Apr 25 '22 11:04 garydgregory