You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by GitBox <gi...@apache.org> on 2020/11/09 20:49:22 UTC
[GitHub] [avro] bellemare opened a new pull request #980: ResolvingGrammarGenerator Union use reader instead of writer schema
bellemare opened a new pull request #980:
URL: https://github.com/apache/avro/pull/980
Make sure you have checked _all_ steps below.
### Jira
- [x ] My PR addresses the following [Avro Jira](https://issues.apache.org/jira/browse/AVRO/) issues and references them in the PR title. For example, "AVRO-1234: My Avro PR"
- https://issues.apache.org/jira/browse/AVRO-2702
### Tests
- [ ] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
### Documentation
- [ ] In case of new functionality, my PR adds documentation that describes how to use it.
- All the public functions and the classes in the PR contain Javadoc that explain what it does
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] bellemare commented on pull request #980: ResolvingGrammarGenerator Union use reader instead of writer schema
Posted by GitBox <gi...@apache.org>.
bellemare commented on pull request #980:
URL: https://github.com/apache/avro/pull/980#issuecomment-724270813
One of the major issues with this PR is that there doesn't appear to be any existing tests that test for it. The reporter issued a comprehensive test (in Kotlin) that _does_ reveal if the patch works (I have verified the code to indeed work), but it seems heavy-handed to implement a single test for what clearly seemed like a mistake in the union coding.
I am interested to see what a committer says about what we should do with regards to testing this, as it could be symptomatic of underlying issues with the ResolvingGrammarGenerator tests, particularly around nesting and strings.
Alternatively, I am fine with porting the single-purpose test from Kotlin to Java and submitting that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] RyanSkraba commented on pull request #980: ResolvingGrammarGenerator Union use reader instead of writer schema
Posted by GitBox <gi...@apache.org>.
RyanSkraba commented on pull request #980:
URL: https://github.com/apache/avro/pull/980#issuecomment-726776234
Hello! This looks important, and I'd like to get a 1.10.1 release candidate soon.
I don't think anyone is _happy_ with missing unit tests, but this does look like the obvious fix. Would it be possible to include the ported Bar.kt test for this specific bug? At the very least, having it in place could encourage other tests to spring up...
Thanks for your contribution!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] RyanSkraba closed pull request #980: AVRO-2702: ResolvingGrammarGenerator Union use reader instead of writer schema
Posted by GitBox <gi...@apache.org>.
RyanSkraba closed pull request #980:
URL: https://github.com/apache/avro/pull/980
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] RyanSkraba commented on pull request #980: AVRO-2702: ResolvingGrammarGenerator Union use reader instead of writer schema
Posted by GitBox <gi...@apache.org>.
RyanSkraba commented on pull request #980:
URL: https://github.com/apache/avro/pull/980#issuecomment-728940373
Included in https://github.com/apache/avro/pull/987 and cherry-picked into branch-1.10. I added the unit test translated from Kotlin. Thanks for your contribution!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org