Skip to content

Conversation

@rxin
Copy link
Contributor

@rxin rxin commented Jul 27, 2018

The diff should be self-explanatory.

@rxin
Copy link
Contributor Author

rxin commented Jul 27, 2018

cc @gatorsmile

cc @hvanhovell why did we expose these types as public Scala APIs? I feel they should not have been public. If they are public, we should have more generic VarcharType and CharType instead.

Something to fix in 3.0?

Copy link
Member

@gatorsmile gatorsmile left a comment

Choose a reason for hiding this comment

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

LGTM

@hvanhovell
Copy link
Contributor

@rxin no, they should not have been public. IMO we should just hide them for 3.0.

@hvanhovell
Copy link
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented Jul 27, 2018

Test build #93684 has finished for PR 21897 at commit 8b954a4.

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

@gatorsmile
Copy link
Member

Thanks! Merged to master.

@asfgit asfgit closed this in 34ebcc6 Jul 27, 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

Development

Successfully merging this pull request may close these issues.

4 participants