-
Notifications
You must be signed in to change notification settings - Fork 99
feat: Support select() with KClass<T> arguments #834
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
Conversation
0f90cec
to
12c3979
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #834 +/- ##
==========================================
+ Coverage 90.55% 90.63% +0.07%
==========================================
Files 337 337
Lines 3567 3597 +30
Branches 222 222
==========================================
+ Hits 3230 3260 +30
Misses 269 269
Partials 68 68 ☔ View full report in Codecov by Sentry. |
Thank you for your help. I'll check it out this weekend. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking of releasing your work as a hotfix next Tuesday, but if you find it difficult to fix the code, please let me know. I'll make some changes and release it.
Thanks for your help!
/** | ||
* Creates a select clause in a select query. | ||
*/ | ||
@SinceJdsl("3.0.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I'm thinking of releasing it as a hotfix, I'd like to modify it to @SinceJdsl(3.5.5)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay :)
@Test | ||
fun `select() with a KClass`() { | ||
// when | ||
val select = queryPart { | ||
select( | ||
BigDecimal::class, | ||
expression1, | ||
).from( | ||
entity1, | ||
) | ||
}.toQuery() | ||
|
||
val actual: SelectQuery<BigDecimal> = select // for type check | ||
|
||
// then | ||
val expected = SelectQueries.selectQuery( | ||
returnType = BigDecimal::class, | ||
distinct = false, | ||
select = listOf(expression1), | ||
from = listOf(entity1), | ||
) | ||
|
||
assertThat(actual).isEqualTo(expected) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like tests are only written for some of the newly created functions. Please add more tests.
And please order the tests and functions correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed these points. Thanks :)
@shouwn I reflected your feedback and revised my code. Please check :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for your help!
Motivation
select(..)
functions usedreified
types for class parameters, making it hard to use them for generic classes even in the presence of class variables (T::class
)Modifications
select(..)
family.Result
Closes
select()
withKClass<T>
arguments #825