Skip to content

Upgrade eslint & config to latest #1179

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.

Sign up for GitHub

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

Merged
bajtos merged 1 commit into from
Dec 6, 2016
Merged

Upgrade eslint & config to latest #1179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/datasource-app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ Article.create(function(e, article) {
// should be able to attach a data source to an existing model
var modelBuilder = new ModelBuilder();

Color = modelBuilder.define('Color', {
const Color = modelBuilder.define('Color', {
name: String,
});

Expand Down
3 changes: 2 additions & 1 deletion lib/connectors/memory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

'use strict';

/* global window:false */
var g = require('strong-globalize')();
var util = require('util');
var Connector = require('loopback-connector').Connector;
Expand Down Expand Up @@ -823,7 +824,7 @@ Memory.prototype.replaceOrCreate = function(model, data, options, callback) {
// Calling _createSync to update the collection in a sync way and
// to guarantee to create it in the same turn of even loop
return self._createSync(model, data, function(err, id) {
if (err) return process.nextTick(function() { cb(err); });
if (err) return process.nextTick(function() { callback(err); });
Copy link
Contributor

@superkhau superkhau Dec 5, 2016

Choose a reason for hiding this comment

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

Why not cb? I still prefer cb over callback unless we've decided to do this all over the place.

Copy link
Contributor

@Amir-61 Amir-61 Dec 5, 2016

Choose a reason for hiding this comment

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

@bajtos @superkhau either one is fine with me; consistency is what I would follow... I did a quick search in lib/connectors/memory.js and it seems most of the methods use callback, so I would personally go for callback in this case; however I agree if I had the option with not lots of callbacks in the file, I would go for cb rather than callback but it is a nitpick.

@superkhau this one actually was a bug since there was no cb in this method and that's why @bajtos changed it to callback.

Copy link
Contributor

@superkhau superkhau Dec 6, 2016

Choose a reason for hiding this comment

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

I'm also fine with either -- I would just like some consistency in our codebase instead of both.

Bug makes sense, but I would like the bugfix in a seperate commit at least so we know its unrelated to the ESLint fixes.

Copy link
Member Author

@bajtos bajtos Dec 6, 2016

Choose a reason for hiding this comment

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

In this patch, I am fixing undefined cb to the correct callback. Let's keep the discussion and (possibly) the refactoring out of scope of this pull request please.

Copy link
Contributor

@superkhau superkhau Dec 6, 2016

Choose a reason for hiding this comment

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

It's a nitpick anyways, 2 commits (one for bug and one for the eslint changes) would've been better for the reviewer to determine WHY the cb/callback thing was changed since the bugfix is not a requirement of ESLint since this PR was for ESLint changes. I'm not saying make a separate PR, just another commit on top of the current changes.

self.saveToFile(id, function(err, id) {
self.setIdValue(model, data, id);
callback(err, self.fromDb(model, data), {isNewInstance: true});
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
},
"devDependencies": {
"async-iterators": "^0.2.2",
"eslint": "^2.13.1",
"eslint-config-loopback": "^4.0.0",
"eslint": "^3.11.1",
"eslint-config-loopback": "^6.0.0",
"mocha": "^2.1.0",
"should": "^8.0.2"
},
Expand Down
1 change: 1 addition & 0 deletions test/basic-querying.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// This test written in mocha+should.js
'use strict';

/* global getSchema:false, connectorCapabilities:false */
Copy link
Contributor

@superkhau superkhau Dec 5, 2016

Choose a reason for hiding this comment

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

We should fix (ie. not use) these globals at some point. ;) That is outside the scope of this PR though.

Copy link
Member Author

@bajtos bajtos Dec 6, 2016

Choose a reason for hiding this comment

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

I agree with both! Should we create an issue to keep track of this work? I am not sure if this problem important enough, there is no point in creating issues that nobody will work for months.

Copy link
Contributor

@superkhau superkhau Dec 6, 2016

Choose a reason for hiding this comment

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

I believe it is important to keep our house as clean as possible and as soon as possible. We should not delay these minor issues for months IMO, but I'll leave that decision up to @ritch. Issue created at #1186 and assigned to @siddhipai

var should = require('./init.js');
var async = require('async');
var db, User;
Expand Down
1 change: 1 addition & 0 deletions test/crud-with-options.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// This test written in mocha+should.js
'use strict';

/* global getSchema:false */
var should = require('./init.js');
var async = require('async');
var db, User, options, filter;
Expand Down
1 change: 1 addition & 0 deletions test/datatype.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// This test written in mocha+should.js
'use strict';

/* global getSchema:false */
var should = require('./init.js');

var db, Model;
Expand Down
1 change: 1 addition & 0 deletions test/default-scope.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// This test written in mocha+should.js
'use strict';

/* global getSchema:false */
var should = require('./init.js');
var async = require('async');

Expand Down
1 change: 1 addition & 0 deletions test/defaults.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// This test written in mocha+should.js
'use strict';

/* global getSchema:false */
var should = require('./init.js');

var db = getSchema();
Expand Down
1 change: 1 addition & 0 deletions test/hooks.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
'use strict';

// This test written in mocha+should.js
/* global getSchema:false */
var should = require('./init.js');

var j = require('../'),
Expand Down
1 change: 1 addition & 0 deletions test/include.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

'use strict';

/* global getSchema:false */
var should = require('./init.js');
var async = require('async');
var assert = require('assert');
Expand Down
2 changes: 2 additions & 0 deletions test/json.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

// This test written in mocha+should.js
'use strict';

/* global getSchema:false */
var should = require('./init.js');

var Schema = require('../').Schema;
Expand Down
2 changes: 1 addition & 1 deletion test/loopback-dl.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1275,7 +1275,7 @@ describe('Model define with relations configuration', function() {
var Post = modelBuilder.define('Post', {userId: Number, content: String});
var User = modelBuilder.define('User', {name: String}, {
relations: {posts: {type: 'hasMany', model: 'Post'},
}});
}});

assert(!User.relations['posts']);
Post.attachTo(ds);
Expand Down
5 changes: 4 additions & 1 deletion test/manipulation.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

// This test written in mocha+should.js
'use strict';

/* global getSchema:false */
var async = require('async');
var should = require('./init.js');

Expand Down Expand Up @@ -442,7 +444,7 @@ describe('manipulation', function() {
.catch(done);
});

it('should save throw error on validation', function() {
it('should save throw error on validation', function(done) {
Person.findOne(function(err, p) {
if (err) return done(err);
p.isValid = function(cb) {
Expand All @@ -454,6 +456,7 @@ describe('manipulation', function() {
'throws': true,
});
}).should.throw(ValidationError);
done();
});
});

Expand Down
2 changes: 2 additions & 0 deletions test/optional-validation.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

// This test written in mocha+should.js
'use strict';

/* global getSchema:false */
var async = require('async');
var should = require('./init.js');
var db, User, options, ModelWithForceId, whereCount = 0;
Expand Down
4 changes: 2 additions & 2 deletions test/persistence-hooks.suite.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1559,7 +1559,7 @@ module.exports = function(dataSource, should, connectorCapabilities) {
});
});

if (!getSchema().connector.replaceById) {
if (!dataSource.connector.replaceById) {
describe.skip('replaceById - not implemented', function() {});
} else {
describe('PersistedModel.prototype.replaceAttributes', function() {
Expand Down Expand Up @@ -2223,7 +2223,7 @@ module.exports = function(dataSource, should, connectorCapabilities) {
});
});

if (!getSchema().connector.replaceById) {
if (!dataSource.connector.replaceById) {
describe.skip('replaceById - not implemented', function() {});
} else {
describe('PersistedModel.replaceOrCreate', function() {
Expand Down
4 changes: 3 additions & 1 deletion test/relations.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

// This test written in mocha+should.js
'use strict';

/* global getSchema:false */
var should = require('./init.js');
var assert = require('assert');
var jdb = require('../');
Expand Down Expand Up @@ -2945,7 +2947,7 @@ describe('relations', function() {
should.not.exist(e);
should.exist(supplier);
supplier.account.update({supplierName: 'Supplier A',
supplierId: sid},
supplierId: sid},
function(err, act) {
should.not.exist(e);
act.supplierName.should.equal('Supplier A');
Expand Down
2 changes: 2 additions & 0 deletions test/schema.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

// This test written in mocha+should.js
'use strict';

/* global getSchema:false */
var should = require('./init.js');

var db = getSchema(), slave = getSchema(), Model, SlaveModel;
Expand Down
2 changes: 2 additions & 0 deletions test/scope.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

// This test written in mocha+should.js
'use strict';

/* global getSchema:false */
var should = require('./init.js');

var db, Railway, Station;
Expand Down
2 changes: 2 additions & 0 deletions test/validations.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

// This test written in mocha+should.js
'use strict';

/* global getSchema:false */
var should = require('./init.js');
var async = require('async');

Expand Down