Skip to content
This repository was archived by the owner on Feb 22, 2024. It is now read-only.

JSHint Refactoring - #4

Merged
raunakkathuria merged 14 commits into
binary-com:masterfrom
borisyankov:master
Jul 16, 2014
Merged

JSHint Refactoring#4
raunakkathuria merged 14 commits into
binary-com:masterfrom
borisyankov:master

Conversation

@borisyankov

Copy link
Copy Markdown

These changes configure JSHint to be used as part of a build process.
Currently it is not included in the 'default' task, but probably should in very near future.

Travis, while full blown CI Server, is currently used to run the 'test' grunt task and not the full build task (this can change in future, or be left like this). Also, a test runner can be added to the 'test' task.

All of the changes are made in order to satisfy JSHint errors, and more future refactorings would be needed.
Also JSHint has some more options that can be enabled, but are kept off for now.

There are 12 more JSHint warnings (down from 345 in the beginning) that are not super obvious how to fix.
This is a good test for Travis too - it should be able to run the test, display the result, but show 'build failed' due to these.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just curious why this was added? I see its for rollbar, just want to know the reason

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I encountered an issue when this.chart.series was undefined.
But it didn't happen consistently.
So I decided adding a custom error in Rollbar, so we can identify it, was a good idea.
Once the cause is determined, I think a proper fix would be either just returning without telling Rollbar about it, or making sure the series property is never undefined.

@raunakkathuria

Copy link
Copy Markdown
Contributor

Seems fine to me overall, only concern is the strict comparison check as we may be depending on type equality check. Will test it however, even it fails we need to fix the code as problem will be in our implementation in that case

@raunakkathuria
raunakkathuria merged commit 2caea83 into binary-com:master Jul 16, 2014
kavehmz pushed a commit that referenced this pull request Jul 11, 2016
mohammad-deriv referenced this pull request in mohammad-deriv/binary-static Aug 22, 2016
mohammad-deriv referenced this pull request in mohammad-deriv/binary-static Aug 23, 2016
negarn referenced this pull request in negarn/binary-static Dec 18, 2017
negarn added a commit that referenced this pull request May 14, 2018
mohammad-deriv pushed a commit that referenced this pull request May 21, 2018
fix issues with nativepicker
ashkanx added a commit that referenced this pull request Nov 8, 2018
easteregg pushed a commit to easteregg/binary-static that referenced this pull request Apr 25, 2019
ashkanx pushed a commit that referenced this pull request Apr 26, 2019
Set granularity and chart type after clean the chart
ashkanx pushed a commit that referenced this pull request May 8, 2019
4p00rv pushed a commit that referenced this pull request Mar 13, 2020
matin-deriv pushed a commit to matin-deriv/binary-static that referenced this pull request Apr 20, 2021
fiona-deriv added a commit to fiona-deriv/binary-static that referenced this pull request Oct 5, 2021
fiona / remove metatrader/types-of-accounts page
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants