AVRO-1723 #79
AVRO-1723 #79
Conversation
Conflicts: lang/java/avro/src/main/java/org/apache/avro/io/parsing/Symbol.java
Conflicts: lang/java/avro/src/main/java/org/apache/avro/io/parsing/Symbol.java
Conflicts: lang/java/avro/src/main/java/org/apache/avro/io/parsing/Symbol.java
|
|
||
| /** | ||
| * | ||
| * @author zoly |
There was a problem hiding this comment.
Nit: we don't use @author tags.
There was a problem hiding this comment.
Another nit: please use 2-spaces for indentation to match the rest of the project.
There was a problem hiding this comment.
Will clean this up, automatically added by the IDE.
…eformat file to 2 space indentation
|
Ryan, I have updated branch with all your suggestions where I see no disagreement. cheers! |
| @@ -1465,7 +1465,10 @@ Schema ReferenceType(): | |||
| name = namespace + "." + name; | |||
| Schema type = names.get(name); | |||
| if (type == null) | |||
| throw error("Undefined name '" + name + "'", token); | |||
| //throw error("Undefined name '" + name + "'", token); | |||
There was a problem hiding this comment.
This should be removed rather than commented out.
There was a problem hiding this comment.
makes sense, will remove.
|
Zoltan, thanks for making those changes. I've added a few more comments from a more in-depth review. |
|
Ryan, I have updated the code with your suggestions, some comments and a few renames, please take another look and let me know if it makes more sense. thank you. |
|
@zolyfarkas, I still don't think it is clear why there are both resolve and intern methods. Can this be done with a single resolve method? There isn't much difference between what they do and there's a lot of duplicated code between the two. |
|
@rdblue, will take some time in the next 2 weeks and simplify this code and attempt to create a traversal utility as you suggested |
|
@rdblue Ryan, I have a first version of a Schema visitor implementation: The visitor interface: A particular implementation: It is still work in progress since I am using it in other use cases and might adapt it to make it easier to use... Sorry for the slow progress, looking forward for your input... |
Conflicts: lang/java/avro/src/test/java/org/apache/avro/TestFixed.java lang/ruby/lib/avro.rb
|
Changed code to use a more generic schema walker... |
Added more tests and fixed a couple of bugs. Also formatted the code
Added a unit-test for forward referencing of named types
Implementation for IDL forward declarations... actually declaration order does not matter anymore...