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

AVRO-4062: Allow leading underscores for names in idl #3178

Merged
merged 4 commits into from
Oct 14, 2024

Conversation

isak-lindbeck
Copy link
Contributor

What is the purpose of the change

This pull request adds support for names with leading underscore in idl. Fixing AVRO-4062
The specification is pretty clear that leading underscore is allowed.

Verifying this change

This change added tests and can be verified as follows:

  • Added java test that parses an example avdl file with leading underscores

Documentation

  • Does this pull request introduce a new feature? no

@github-actions github-actions bot added the Java Pull Requests for Java binding label Sep 26, 2024
@isak-lindbeck
Copy link
Contributor Author

Thanks for the approve @martin-g, is there something more needed to do to get this PR merged?
Since this fixes a bug introduced in 1.12 should I create another PR targeting the 1.12 branch?

@martin-g martin-g requested a review from opwvhk October 14, 2024 12:14
@opwvhk
Copy link
Contributor

opwvhk commented Oct 14, 2024

Looks good, especially due to the added test case.

I also checked the spec on names, en this fixes a bug in allowed names. Thank you!

@opwvhk opwvhk merged commit ed6d4b7 into apache:main Oct 14, 2024
8 checks passed
martin-g pushed a commit that referenced this pull request Oct 14, 2024
* AVRO-4062 [java] Fix broken test data

* AVRO-4062 [java] Make maven run unit test for IdlReader

The test is not run automatically unless the class naming convention is followed

* AVRO-4062 Allow leading underscore for names in idl

* AVRO-4062 [java] Add test for leading underscore in names

---------

Co-authored-by: Isak Lindbeck <[email protected]>
(cherry picked from commit ed6d4b7)
@martin-g
Copy link
Member

Since this fixes a bug introduced in 1.12 should I create another PR targeting the 1.12 branch?

I have cherry-picked it!
Thank you, @isak-lindbeck !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java Pull Requests for Java binding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants