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

Package name casing inconsistencies #2102

Closed
AnmAtAnm opened this issue Nov 2, 2018 · 3 comments
Closed

Package name casing inconsistencies #2102

AnmAtAnm opened this issue Nov 2, 2018 · 3 comments

Comments

@AnmAtAnm
Copy link
Contributor

AnmAtAnm commented Nov 2, 2018

The Blockly namespace is capitalized.
The Blockly.Xml namespace is also capitalized.
The Blockly.utils and Blockly.Xml.utils used a mix of capitalized and lowercase.

Google Style Guide recommends lowerCamelCase for each part of a package name.

(The likelihood of fixing these are probably zero, due to backward compatibility needs. Still, it is worth highlighting to prevent further inconsistencies.)

@NeilFraser
Copy link
Contributor

NeilFraser commented Jul 8, 2019

To clarify, a name should start with lowercase if it is just a namespace, but should start uppercase if it is a constructor. Thus goog and goog.math are lowercase, but goog.math.Coordinate starts uppercase since the latter is a constructor.

In our case Blockly should have been lowercase, but that's impossible to change now. Blockly.inject is correctly lowercase, Blockly.FieldAngle is correctly uppercase, and Blockly.Touch is incorrectly uppercase.

A list of currently incorrect names in core are:

Blockly
Blockly.BlockAnimations
Blockly.Blocks
Blockly.ContextMenu
Blockly.Css
Blockly.DropDownDiv
Blockly.Events
Blockly.Extensions
Blockly.Msg
Blockly.Procedures
Blockly.Themes
Blockly.Themes.Classic
Blockly.Themes.HighContrast
Blockly.Themes.Modern
Blockly.Tooltip
Blockly.Touch
Blockly.Variables
Blockly.VariablesDynamic
Blockly.WidgetDiv
Blockly.Xml

Not sure if there are any here that can be easily renamed without breaking 3rd party stuff.

@NeilFraser
Copy link
Contributor

After discussion with the team, the only item on this list we felt comfortable changing was Blockly.BlockAnimations > Blockly.blockAnimations. The rest failed the cost-benefit analysis.

@NeilFraser
Copy link
Contributor

For the record:

  • we all agreed that Blockly.Themes should be Blockly.themes
  • we could not agree on whether Blockly.Themes.Classic should become Blockly.themes.classic or Blockly.themes.CLASSIC
  • we all agreed that the question was moot since any change to themes would be too disruptive.

Closing issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants