search

phetsims / phet-core

Core utilities used by all PhET simulations.
MIT License
8 stars 6 forks source link

How should casing work for Enumerations? #43

Closed samreid closed 3 years ago

samreid commented 5 years ago

In https://github.com/phetsims/phet-core/issues/42#issue-371259394 @pixelzoom said:

In the examples and current usages, the name is camel case. But it's an instance/constant, not a class/type. Are we making an exception to PhET naming conventions here?

pixelzoom commented 5 years ago

FYI, I've followed precedent and have been using class-like names for Enumerations. See examples in Gas Properties.

samreid commented 5 years ago

@jonathanolson also said:

It is "acting" like a type defined by being one of the specified string values, so it's named like a type.

samreid commented 5 years ago

But it's an instance/constant, not a class/type

We are discussing making this into a true type in #50. But even if we don't make it a true type, we can use the convention that we case it like a type, as mentioned in https://github.com/phetsims/phet-core/issues/43#issuecomment-444337288 and 👍 in the preceding comment. @pixelzoom can this issue be closed?

pixelzoom commented 5 years ago

It can be closed when the rest of the development team is made aware of the convention, either at a developer meeting, or (more appropriately) in a style guide or code review doc.

samreid commented 5 years ago

Can this be checked with a lint rule?

jonathanolson commented 4 years ago

I treat the enumeration as defining a type, so that we can document things as being "that type".

It seems the remaining task here is to:

  1. Discuss with the team (or inform), so I'm tagging for developer meeting.
  2. Decide whether we should have a lint rule.

We don't have lint rules for naming classes or most other things, so I'm not too worried either way on (2).

jessegreenberg commented 4 years ago

Discuss with the team (or inform), so I'm tagging for developer meeting.

This was discussed with the team in #42.

Decide whether we should have a lint rule.

Yes, this should be a lint rule. @jessegreenberg (with help from @Denz1994) will create the lint rule.

jessegreenberg commented 3 years ago

I added a new rule for this in https://github.com/phetsims/chipper/commit/0175a8390b30ab1decd24f3f87ba74115eb4db04, and fixed the couple of places where it found this convention was not being followed. All were in testing code. Closing.