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

Event String names should be consistently cased #1435

Closed
dsparacio opened this issue Jun 3, 2016 · 4 comments
Closed

Event String names should be consistently cased #1435

dsparacio opened this issue Jun 3, 2016 · 4 comments
Assignees
Milestone

Comments

@dsparacio
Copy link
Contributor

Some history
Michael and My intention was CoreEvents.js are all internal client events that a player will not need access to, if we want something to be documented and external it can be moved from CoreEvnets to MediaPlayerEvents with no other changes. This is because all events are pushed into Events.js which the client uses internally. In addition events marked with prefix string "public_" E.g. ProtectionEvents.js, this will be programmatically moved to MediaPlayerEvents as well so you can access them in one place at the player level.

We need to pick one style and apply. camelCased VS lowercased

@dsparacio dsparacio added this to the 2.2.0 milestone Jun 3, 2016
@dsparacio dsparacio self-assigned this Jun 3, 2016
@boushley
Copy link
Member

Do we have any opinion on the style used? I can see arguments for either direction.

cameCase is a bit easier to read and uderstand.
lowercased is more in line with native browser events.

Personally I lean more towards camelCase because it can take a second or two to stare at the lowercased string and figure out exactly what is being said, where camelCase is much easier to read quickly.

@dsparacio dsparacio modified the milestones: v2.3.0, 2.2.0 Jul 5, 2016
@dsparacio dsparacio modified the milestones: v2.4.0, v2.3.0 Sep 6, 2016
@dsparacio
Copy link
Contributor Author

There are more camalCased and easier to read so all strings have been converted. I also moved a few events from Core to MediaPlayer which will not affect any internal code or existing players.

dsparacio pushed a commit to dsparacio/dash.js that referenced this issue Sep 8, 2016
@boushley
Copy link
Member

boushley commented Sep 8, 2016

👍

dsparacio pushed a commit that referenced this issue Sep 8, 2016
@dsparacio
Copy link
Contributor Author

fixed with PR #1582

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