Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Boolean Nulls/Refactor Time Adapters/Adapter Tests #55

Merged
merged 24 commits into from
Jun 19, 2023

Conversation

SentryMan
Copy link
Collaborator

@SentryMan SentryMan commented Jun 15, 2023

As the title says.

  • Fixes null handling with boolean values
  • fix Min/Max adapters casting issue (can't cast Object to long if it's an int)
  • refactor FuturePastAdapter to be simple and use a referenceClock for tolerance (somewhat similar algo to how hibernate does it)
  • no longer uses reflection to check array length

@SentryMan
Copy link
Collaborator Author

hmm, it seems we will need a way to set the tolerance for the time adapters

@SentryMan SentryMan changed the title Fix Boolean Nulls/Add Some Adapter Tests Fix Boolean Nulls/Refactor Time Adapters/Add Some Adapter Tests Jun 15, 2023
@SentryMan SentryMan changed the title Fix Boolean Nulls/Refactor Time Adapters/Add Some Adapter Tests Fix Boolean Nulls/Refactor Time Adapters/Adapter Tests Jun 15, 2023
@SentryMan
Copy link
Collaborator Author

Yeah, I'm good with this

@SentryMan
Copy link
Collaborator Author

oh wait I forgot the number ones

@SentryMan
Copy link
Collaborator Author

should be good now

return true;
}

if (pattern.test(value.toString())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong / inverted boolean ? Kind of expecting a test to fail ? Hmm.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, there is the negate() there ... not sure how the tests previously passed?

@rbygrave rbygrave merged commit 72c9980 into avaje:main Jun 19, 2023
@SentryMan SentryMan deleted the adapter-tests branch June 19, 2023 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants