-
Notifications
You must be signed in to change notification settings - Fork 948
Enable linting and fix lint issues for app, util and logger #1845
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
Conversation
packages/util/src/crypt.ts
Outdated
@@ -154,34 +154,34 @@ export const base64 = { | |||
* | |||
* @param input An array of bytes (numbers with | |||
* value in [0, 255]) to encode. | |||
* @param opt_webSafe Boolean indicating we should use the | |||
* @param optwebSafe Boolean indicating we should use the |
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.
Did you mean to get rid of the opt_
entirely? https://engdoc.corp.google.com/eng/doc/devguide/js/styleguide.md?cl=head#features-functions-default-parameters
If not, it should still be camelCase I think.
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.
Good catch. Updated to webSafe
.
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.
I think they mean option
instead of optional
to which the google styleguide references, but it is redundant.
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.
Looks good, just some comments based on the Google JS style guide which I'm not sure if it applies.
compress_(buf, opt_offset?) { | ||
if (!opt_offset) { | ||
opt_offset = 0; | ||
compress_(buf, optOffset?): void { |
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 just be offset
if we are following Google style guide (see previous comment)
@@ -190,53 +190,53 @@ export class Sha1 { | |||
this.chain_[4] = (this.chain_[4] + e) & 0xffffffff; | |||
} | |||
|
|||
update(bytes, opt_length?) { | |||
update(bytes, optLength?): void { |
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.
Same as above, length
?
No description provided.