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

Rename classes with "Extensions" in name. #1022

Closed
dsparacio opened this issue Jan 15, 2016 · 5 comments
Closed

Rename classes with "Extensions" in name. #1022

dsparacio opened this issue Jan 15, 2016 · 5 comments
Assignees
Milestone

Comments

@dsparacio
Copy link
Contributor

We have moved away from the idea that you can only replace objects with the word Extensions in the file name.

VideoModelExtensions can be removed and method move do VideoModel.

@dsparacio dsparacio added this to the 2.0.0 milestone Jan 15, 2016
@dsparacio dsparacio self-assigned this Jan 15, 2016
@dsparacio dsparacio modified the milestones: 2.1.0, 2.0.0 Feb 5, 2016
@dsparacio
Copy link
Contributor Author

Hello all. With 2.0 we have changed how you extend objects and the idea of marking a class with the name extension does not make much sense anymore. So I propose the following file name changes to happen before we release. Please take some time to review the changes and let me know ASAP if you have issue or suggestions with any of the changes. Very open minded to the name and location just want to get the extensions out of name and removed that folder. There a couple other moves to and from utils that seems to make sense to do now as well.

streaming/extensions:
extensions/MediaSourceExtensions.js ----> controllers/MediaSourceController.js
extensions/RequestModifierExtensions.js ----> utils/RequestModifier.js
extensions/SourceBufferExtensions.js ----> streaming/SourceBuffer.js
extensions/TextTrackExtensions.js ----> streaming/TextTracks.js
extensions/VideoModelExtensions.js ----> one method will be merged into VideoModel and entire class will be deleted.

streaming/utils:
utils/VirtualBuffer.js ----> streaming/VirtualBuffer.js

dash/extensions:
extensions/BaseURLExtensions.js ----> dash/BaseURLLoader.js OR controllers/BaseURLContorller.js
extensions/DashManifestExtensions.js ----> dash/DashManifest.js
extensions/DashMetricsExtensions.js ----> dash/DashMetrics.js
extensions/FragmentExtensions.js ----> dash/utils/FragmentedTextUtil.js

dash ---> dash/util:
dash/TimelineConverter.js ----> dash/utils/TimelineConverter.js

protection:
protection/ProtectionExtensions.js ---> ProtectionKeyController.js or merge into protection controller. The latter being a big change near a release.

@LloydW93
Copy link
Member

LloydW93 commented Feb 8, 2016

I think renaming SourceBufferExtensions to SourceBuffer could cause confusion - to make SourceBufferController makes more sense, as it creates and manipulates SourceBuffers.

@davemevans
Copy link
Contributor

I think BaseURLLoader or BaseURLController are misleading. SegmentBaseLoader/Controller?

@dsparacio
Copy link
Contributor Author

I think both suggestions are good and I will use over my original names. Thanks guys.

dsparacio pushed a commit that referenced this issue Feb 9, 2016
@dsparacio
Copy link
Contributor Author

Fixed with PR #1145.

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

3 participants