-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
services type fixes #4249
services type fixes #4249
Conversation
@@ -219,6 +219,7 @@ export class Log { | |||
* @param {...*} var_args Arguments substituted into %s in the message. | |||
* @return {T} The value of shouldBeTrueish. | |||
* @template T | |||
* @throws {Error} |
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.
Lets not spam the code base with these.
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.
should i remove here then? didnt realize the compiler doesnt even use it besides detecting side effects
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.
Are you sure it is being used at all? I don't think so. Probably on externs
only.
On Jul 28, 2016 10:21 AM, "erwin mombay" notifications@github.com wrote:
In src/log.js
#4249 (comment):@@ -219,6 +219,7 @@ export class Log {
* @param {...*} var_args Arguments substituted into %s in the message.
* @return {T} The value of shouldBeTrueish.
* @template T
- * @throws {Error}
should i remove here then? didnt realize the compiler doesnt even use it
besides detecting side effects—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/ampproject/amphtml/pull/4249/files/7ef905fb26fde6d8e3f030df76a6c780dba8d889#r72581746,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAFeT6itAXu5wks1ywbO7YO617Kpzhzvks5qaGZ9gaJpZM4JWvb9
.
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.
you are right, that is the case. "The type checker does not currently use this information. It is only used to figure out if a function declared in an externs file has side effects"
524bd64
to
846769d
Compare
@@ -124,7 +124,7 @@ export class Viewer { | |||
/** @private {number} */ | |||
this.prerenderSize_ = 1; | |||
|
|||
/** @private {string} */ | |||
/** @private {ViewportType} */ |
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.
!ViewportType
?
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.
yep, thanks.
66ba186
to
4473a3d
Compare
4473a3d
to
f5f301f
Compare
@cramforce PTAL |
f5f301f
to
5ff65fa
Compare
LGTM Idea: Comment the "Number of errors" after the change in each of these changes :) |
5ff65fa
to
e49b11c
Compare
178 errors |
* more xhr type fixes * v-services type fixes
* more xhr type fixes * v-services type fixes
No description provided.