datafaker-net / datafaker

Generating fake data for the JVM (Java, Kotlin, Groovy) has never been easier!
https://www.datafaker.net
Apache License 2.0
1.09k stars 151 forks source link

Unify `TimeAndDate` provider API #1282

Closed valfirst closed 1 week ago

valfirst commented 1 week ago

TimeAndDate#past methods use minimum input parameter of long type, but TimeAndDate#future methods - of int type. This change unifies API: minimum input parameter is always of long type

what-the-diff[bot] commented 1 week ago

PR Summary

codecov-commenter commented 1 week ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.01%. Comparing base (b37c566) to head (eee4d67). Report is 202 commits behind head on main.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1282 +/- ## ============================================ - Coverage 92.35% 92.01% -0.34% - Complexity 2821 3087 +266 ============================================ Files 292 310 +18 Lines 5609 6025 +416 Branches 599 627 +28 ============================================ + Hits 5180 5544 +364 - Misses 275 330 +55 + Partials 154 151 -3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bodiam commented 1 week ago

Good point, thanks for this!

kingthorin commented 1 week ago

I know we just made these date/time changes but technically this breaks the public API (people will have to update their code to accommodate). The methods should have been deprecated and new methods implemented (the old could even call the new and cast the params too long).

[For my two cents, I know others on this project don't see it as strictly as I do 😉]

bodiam commented 1 week ago

Hi @kingthorin

people will have to update their code to accommodate

Why would consumers need to change their code? It's a change from an int to a long, from the callers perspective nothing should change (hence no unit tests were changed either).

(and yes, if we'd follow semver to the letter, we'd be on Datafaker 16 by now ;-) )

kingthorin commented 1 week ago

Nope you're right, Java treats a number as a number in this case. I had been thinking, ex: 3 ➡️ 3L, but Java doesn't force that.

bodiam commented 1 week ago

It's a good point though. I thought there was something, that if you pass in a Long (or Int), and we change the method signature without recompiling, the code cannot find the signature anymore or so. I don't remember the exact case for this.

I do think one day we should use something like this: https://lvc.github.io/japi-compliance-checker/

kingthorin commented 1 week ago

In another project I work on they use japicmp.