Skip to content

Conversation

@ark-1
Copy link
Contributor

@ark-1 ark-1 commented Apr 2, 2024

Fixes #640

@ark-1 ark-1 added the bug Something isn't working label Apr 2, 2024
@ark-1 ark-1 requested a review from Jolanrensen April 2, 2024 15:59
@ark-1 ark-1 self-assigned this Apr 2, 2024
@koperagen
Copy link
Collaborator

It was done on purpose, see the test. I think this should be fixed on kernel side, if possible. It's not obvious why when our function returns null no other converters are called

@ark-1
Copy link
Contributor Author

ark-1 commented Apr 2, 2024

I will discuss changing this behavior on the kernel side. Still, your case is already supported by a different API, so I suggest changing it nevertheless.

@koperagen
Copy link
Collaborator

Cool :) Then, run assemble and add generated sources to the commit please. No other questions from me

@Jolanrensen
Copy link
Collaborator

Yes, we need to make sure this #401 still works

@Jolanrensen
Copy link
Collaborator

All tests pass, also the ones introduced by #401, so should be good to go :)

@ark-1
Copy link
Contributor Author

ark-1 commented Apr 13, 2024

The problem with the proposed code that it didn't guarantee the ordering of checks and conversions.
I have rewritten the code to use full FieldHandler API, that has a separate accepts method.

@ark-1 ark-1 merged commit 195a305 into master Apr 15, 2024
@Jolanrensen Jolanrensen deleted the ark/fix-640 branch April 15, 2024 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

4 participants