Fix eslint in 3D-tiles branch #5372
Conversation
| @@ -111,7 +111,7 @@ define([ | |||
| var data = this._data; | |||
| var candidate = -1; | |||
|
|
|||
| while (true) { | |||
| while (true) { // eslint-disable-line no-constant-condition | |||
There was a problem hiding this comment.
I'm not sure I like the idea of while(true) in our code to begin with, but I guess this can be discussed in the 3d-tiles PR itself.
| @@ -1198,7 +1198,7 @@ define([ | |||
| //>>includeStart('debug', pragmas.debug); | |||
| throw new DeveloperError('Operator "-" requires a vector or number argument. Argument is ' + left + '.'); | |||
There was a problem hiding this comment.
@lilleyse Shouldn't these be RuntimeError or has style validation already happened at this point? Assuming we change them to runtime errors, that would get rid of the elint error as well because the below returns would never happen.
Even if they are runtime errors, this code makes no sense, we know left is not a number from the above failed check but we are returning -left anyway?
There was a problem hiding this comment.
They could be RuntimeError. I guess my usual thought process is that "the developer wrote a bad style, so throw a developer error", but maybe I need to rethink that.
Returning -left is a remnant from the time when the styling language was less type strict. It would let a style like -"5" actually become -5. I can fix that throughout now that the rules have changed.
There was a problem hiding this comment.
Sounds good.
In general, while Cesium is aggressive with developer errors in API calls, anything that deals with externally loading data should normally use RuntimeError instead (we need to do better about this all around).
|
Since the original intent of this PR was to fix the eslint errors in Thanks @hpinkos! |
No description provided.