Skip to content
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

Update dependencies. Replace JSHint with ESLint. Remove grunt #61

Merged
merged 4 commits into from
Dec 29, 2019
Merged

Update dependencies. Replace JSHint with ESLint. Remove grunt #61

merged 4 commits into from
Dec 29, 2019

Conversation

kibertoad
Copy link
Contributor

Since Grunt is pretty much considered legacy now, I've replaced it with direct npm script invocation.
Same with JSHint - ESLint is up-to-date alternative.

@@ -202,9 +202,10 @@ describe("yolo", function () {

var onReadable = function () {
// Readable is emitted as soon a row is received and parsed
var row;
while (row = this.read()) {
var row = this.read();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ESLint is unhappy about assignment within condition.

@@ -103,6 +103,7 @@ function _getSelect() {
if (_.has(this._grouped, "aggregate")) {
_.each(this._grouped.aggregate, function (aggregate) {
// TODO add.. more.. aggregates
var key, val;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ESLint is unhappy about redefining previously defined key, val variables, hence I extracted definition to a common scope.

@kibertoad
Copy link
Contributor Author

@UnbounDev Would you consider taking a look?

@austinbrown-okta austinbrown-okta self-requested a review December 29, 2019 00:40
Copy link
Contributor

@austinbrown-okta austinbrown-okta left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution @kibertoad and apologies that it slipped my notice for so long!

@austinbrown-okta austinbrown-okta merged commit 424843f into azuqua:master Dec 29, 2019
@kibertoad kibertoad deleted the chore/update-dependencies branch December 29, 2019 11:31
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.

2 participants