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

Component styles #2105

Closed
wants to merge 2 commits into from
Closed

Component styles #2105

wants to merge 2 commits into from

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Apr 30, 2015

component.js will now pass latest version of videojs-standard.
There are still a few warning that it issues, though. These will not fail the build.

standard: Use VideoJS JavaScript Standard Style (https://github.com/videojs/standard)
  /Users/gkatsevman/p/video.js/src/js/component.js:206:0: Line 206 exceeds the maximum line length of 90.
  /Users/gkatsevman/p/video.js/src/js/component.js:309:0: Line 309 exceeds the maximum line length of 90.
  /Users/gkatsevman/p/video.js/src/js/component.js:310:0: Line 310 exceeds the maximum line length of 90.
  /Users/gkatsevman/p/video.js/src/js/component.js:311:0: Line 311 exceeds the maximum line length of 90.
  /Users/gkatsevman/p/video.js/src/js/component.js:321:0: Line 321 exceeds the maximum line length of 90.
  /Users/gkatsevman/p/video.js/src/js/component.js:328:0: Line 328 exceeds the maximum line length of 90.
  /Users/gkatsevman/p/video.js/src/js/component.js:457:36: Unexpected use of undefined.
  /Users/gkatsevman/p/video.js/src/js/component.js:677:0: Line 677 exceeds the maximum line length of 90.
  /Users/gkatsevman/p/video.js/src/js/component.js:725:0: Line 725 exceeds the maximum line length of 90.
  /Users/gkatsevman/p/video.js/src/js/component.js:861:0: Line 861 exceeds the maximum line length of 90.
  /Users/gkatsevman/p/video.js/src/js/component.js:871:16: Unexpected use of undefined.
  /Users/gkatsevman/p/video.js/src/js/component.js:873:26: Comparing to itself is potentially pointless.
  /Users/gkatsevman/p/video.js/src/js/component.js:886:0: Line 886 exceeds the maximum line length of 90.
  /Users/gkatsevman/p/video.js/src/js/component.js:912:4: Unexpected todo comment.
  /Users/gkatsevman/p/video.js/src/js/component.js:945:0: Line 945 exceeds the maximum line length of 90.
  /Users/gkatsevman/p/video.js/src/js/component.js:985:4: Unexpected todo comment.
  /Users/gkatsevman/p/video.js/src/js/component.js:1075:0: Line 1075 exceeds the maximum line length of 90.
  /Users/gkatsevman/p/video.js/src/js/component.js:1160:0: Line 1160 exceeds the maximum line length of 90.
  /Users/gkatsevman/p/video.js/src/js/component.js:1170:0: Line 1170 exceeds the maximum line length of 90.


// The component might be the player itself and we can't pass `this` to super
if (!player && this.play) {
this.player_ = player = this;
this.player_ = player = this; // eslint-disable-line
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how you would ignore a single line if we really want to keep it and it breaks the rules.
I decided to do it for this line because it's too much of a pain to figure out a good way to refactor this so what is happening is still clear.

@heff
Copy link
Member

heff commented May 1, 2015

LGTM!

This now passes the style linting as followed by videojs-standard.
There are still some warnings but no errors.
@gkatsev gkatsev closed this in 43a1429 May 1, 2015
@gkatsev gkatsev deleted the component-styles branch August 12, 2015 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants