Skip to content

Conversation

@m1ga
Copy link
Contributor

@m1ga m1ga commented Dec 14, 2022

  • clean analytics file (still kept it since I'm not sure if other files use it)
  • clean auth file (still kept it since I'm not sure if other files use it)
  • fixed link to Oracle download and added OpenJDK link
  • updated node packages
  • removed auth/analytics test
  • fixed some of the warnings in the tests
  • added allowEmptyCatch rule to ignore empty blocks

Copy link
Contributor

@cb1kenobi cb1kenobi left a comment

Choose a reason for hiding this comment

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

This PR is a great first step! Thanks for working on this.

"sprintf": "^0.1.5",
"temp": "~0.9.4",
"uuid": "~8.3.2",
"uuid": "~9.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd have to test this, but I'm hesitant to blind merge a major bump.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

only tested it on Linux. btw: I don't see where it is used

}

return cachedStatus = result;
exports.login = function login(_args) {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Elsewhere you had to put a comment in the body of a scope to I believe make lint happy. I'm curious why linting complained elsewhere, but not here? FWIW, I do prefer the empty body as you have it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer this too, even for the code blocks in an if or catch. We could add allowEmptyCatch to ignore this warning

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I say we add allowEmptyCatch (which I didn't know existed!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done 👍

Copy link
Contributor

@cb1kenobi cb1kenobi left a comment

Choose a reason for hiding this comment

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

LGTM

@cb1kenobi cb1kenobi merged commit ff9b291 into master Dec 15, 2022
@m1ga m1ga deleted the 221214_jdk branch December 15, 2022 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants