-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add string constants file #1996
Add string constants file #1996
Conversation
|
||
class StringMatcher extends BaseMatcher { | ||
constructor() { | ||
super( | ||
(attr, nodeName) => { | ||
const stringAttrsInElements = { | ||
'MPD': [ 'id', 'profiles' ], | ||
'Period': [ 'id' ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In ES6, these keys can also be replaced with DashConstant constants by wrapping them in [ ]. See https://github.com/Dash-Industry-Forum/dash.js/blob/development/src/streaming/XHRLoader.js#L63 for an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meant to comment on the addition rather than the removal but you get the idea ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bbcrddave for the tip, I didn't know it was possible.
I have made the modification
f69c546
to
96e80ef
Compare
f910a40
to
38960fe
Compare
…imum allowed is 30. '
38960fe
to
1b5eb92
Compare
Hi guys,
The goal of this PR is to remove hardcoded strings, and to add constants files.
I have added 3 constants files :
The idea is to centralized strings constants, so that they can be easily used and of course modified if needed.
Jérémie