Skip to content

Conversation

@AngersZhuuuu
Copy link
Contributor

What changes were proposed in this pull request?

Current checkstyle checking folder can't cover all folder.
Since for support multi version hive, we have some divided hive folder.
We should check it too.

Why are the changes needed?

Fix build bug

Does this PR introduce any user-facing change?

NO

How was this patch tested?

NO

@AngersZhuuuu
Copy link
Contributor Author

cc @srowen @wangyum @dongjoon-hyun

@wangyum
Copy link
Member

wangyum commented Nov 4, 2019

ok to test

pom.xml Outdated
<sourceDirectories>
<directory>${basedir}/src/main/java</directory>
<directory>${basedir}/src/main/scala</directory>
<directory>v${hive.version.short}/src/main/java</directory>
Copy link
Member

Choose a reason for hiding this comment

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

v${hive.version.short}/src/main/java -> ${basedir}/v${hive.version.short}/src/main/java?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

v${hive.version.short}/src/main/java -> ${basedir}/v${hive.version.short}/src/main/java?

done

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

I think it's OK, if there are few style checks the fix. The minor issue is that some of this code is copied from Hive right? so if we modified it a lot it might be harder to update. But minor changes like this are no big deal.

@AngersZhuuuu
Copy link
Contributor Author

I think it's OK, if there are few style checks the fix. The minor issue is that some of this code is copied from Hive right? so if we modified it a lot it might be harder to update. But minor changes like this are no big deal.

Not all copied code from hive. sql/core module have some java code in folder v1.2.1/v2.3.5 .

@SparkQA
Copy link

SparkQA commented Nov 4, 2019

Test build #113210 has finished for PR 26385 at commit 5681e0f.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Nov 4, 2019

Test build #113211 has finished for PR 26385 at commit c998060.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Yes. Right. +1, LGTM. Thank you, @AngersZhuuuu , @srowen , @wangyum .
Merged to master.

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.

5 participants