Skip to content

Conversation

@openinx
Copy link
Member

@openinx openinx commented Jun 8, 2020

When I try to pull request the flink parquet reader & writer (#1096), I scanned the unit test and found that the RandomData was copied and pasted in several unit test files. So I made this pull request to put it into the utility class and let the unit test files share it.

@openinx
Copy link
Member Author

openinx commented Jun 9, 2020

Hi @rdblue , Mind to take a look ? the next flink patch will depends on this patch (so that we could reuse parts of the random utility methods), thanks. https://github.com/openinx/incubator-iceberg/commit/72cf683656dd993334742f0edc514e0289f543f8

@rdblue rdblue closed this Jun 9, 2020
@rdblue rdblue reopened this Jun 9, 2020
public class RandomUtil {

private RandomUtil() {

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: no need for a blank line here.

import org.apache.iceberg.types.Type;
import org.apache.iceberg.types.Types;

public class RandomUtil {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this class should be in src/main. Can you move it to src/test?

@rdblue
Copy link
Contributor

rdblue commented Jun 9, 2020

Looks good other than the class in the wrong source tree.

@rdblue rdblue merged commit 35dd7de into apache:master Jun 10, 2020
@rdblue
Copy link
Contributor

rdblue commented Jun 10, 2020

Merged. Thanks, @openinx!

@openinx openinx deleted the reuse-random-data branch June 11, 2020 02:08
cmathiesen pushed a commit to ExpediaGroup/iceberg that referenced this pull request Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants