Skip to content

[SPARK-29866][SQL][SS][ML] Upper case enum values#26487

Closed
MaxGekk wants to merge 19 commits intoapache:masterfrom
MaxGekk:enum-upper-case
Closed

[SPARK-29866][SQL][SS][ML] Upper case enum values#26487
MaxGekk wants to merge 19 commits intoapache:masterfrom
MaxGekk:enum-upper-case

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Nov 12, 2019

What changes were proposed in this pull request?

In the PR, I propose to rename enum values and upper case them for unification.

Why are the changes needed?

To follow common approach and unify naming of enum values.

Does this PR introduce any user-facing change?

No

How was this patch tested?

By existing tests

@MaxGekk
Copy link
Member Author

MaxGekk commented Nov 12, 2019

@srowen @dongjoon-hyun Could you review this PR, please.

@SparkQA
Copy link

SparkQA commented Nov 12, 2019

Test build #113646 has finished for PR 26487 at commit 3b11328.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

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.

Although I'd like a suggested standard, I don't think this is worth changing retroactively, especially where it's user-visible. Scala enums seem to use CamelCase. Java enums use ALL_CAPS mostly for parity with the 'constants' naming convention, even though they act a little more like class instances.

@dongjoon-hyun
Copy link
Member

I also agree with @srowen . This is not worth of changing the existing code.

@MaxGekk MaxGekk closed this Nov 13, 2019
@MaxGekk MaxGekk deleted the enum-upper-case branch June 5, 2020 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants