Skip to content

Add converters for simple type projections #65

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

Closed

Conversation

uaihebert
Copy link
Contributor

partial solution for #41

Copy link
Member

@mp911de mp911de left a comment

Choose a reason for hiding this comment

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

That's a great start. We haven't discussed yet which types to convert. We should include LocalDate, LocalDateTime, LocalTime, OffsetDateTime and ZonedDateTime. I'm not sure about Date yet, InetAddr is not supported by R2DBC.


@Override
public Boolean convert(Row row) {
return (Boolean) row.get(0);
Copy link
Member

Choose a reason for hiding this comment

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

Use row.get(0, Boolean.class) to avoid downcasting.

This also applies for all other calls (except for theRowToNumberConverterFactory).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

awesome. I will do this refactorings and create a test for the converters.

@uaihebert uaihebert force-pushed the feature/add-row-converters branch from c1b350b to 61f2966 Compare February 22, 2019 11:37
@uaihebert
Copy link
Contributor Author

hellp @mp911de PR is updated.

@mp911de
Copy link
Member

mp911de commented Feb 26, 2019

Thanks a lot. The code looks good. Care to add a bit of Javadoc? Have a look into the other store modules that provide similar converters for inspiration.

@uaihebert uaihebert force-pushed the feature/add-row-converters branch from 61f2966 to 5bdaafe Compare February 26, 2019 21:59
@uaihebert
Copy link
Contributor Author

done.

Let me know if there is anything else that I should do here.

@mp911de
Copy link
Member

mp911de commented Feb 27, 2019

Looks good. We now should wait until #62 is merged to avoid further merge conflicts.

@mp911de mp911de added type: enhancement A general enhancement status: blocked An issue that's blocked on an external project change labels Feb 27, 2019
@mp911de mp911de added this to the 1.0 M2 milestone Feb 27, 2019
@mp911de mp911de removed the status: blocked An issue that's blocked on an external project change label Mar 7, 2019
@mp911de mp911de changed the title add a first row data converter Add converters for simple type projections Mar 7, 2019
mp911de pushed a commit that referenced this pull request Mar 10, 2019
Original pull request: #65.
mp911de added a commit that referenced this pull request Mar 10, 2019
Rename RowDataConverter to R2dbcConverters. Introduce R2dbcSimpleTypeHolder. Apply custom conversions check in MappingR2dbcConverter. Extend tests.

Original pull request: #65.
@mp911de
Copy link
Member

mp911de commented Mar 10, 2019

That's merged and polished now. Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants