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-2267 Removed copies of RandomData #385

Merged
merged 2 commits into from Nov 20, 2018
Merged

Conversation

thiru-mg
Copy link
Contributor

  • Unified three near identical copies of RandomData.java into a single file.
  • Main java code of tools module used to have a dependency on test code of avro module. It is not a good idea to have release code include test code. Broken the link, by placing the random data generation in avro module's util package.

@thiru-mg thiru-mg self-assigned this Nov 18, 2018
Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

Thanks @thiru-apache for picking this up. Looks good to me, with one remark.

System.out.println("Usage: RandomData <schemafile> <outputfile> <count> [codec]");
System.exit(-1);
}
Schema sch = Schema.parse(new File(args[0]));
Schema sch = new Schema.Parser().parse(new File(args[0]));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a static function is nicer here. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the static method is marked as deprecated (which is a wrapper around the non-static one), and the non-static version should be used instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nandorKollar Yes, you are right. The IDE complains about it and that is the reason I changed it.

@Fokko Fokko changed the title Removed copies of RandomData AVRO-2267 Removed copies of RandomData Nov 20, 2018
@Fokko Fokko merged commit eb06f71 into apache:master Nov 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants