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

Use eslint-config-metarhia and fix linting issues #379

Merged
merged 1 commit into from
Nov 1, 2018

Conversation

belochub
Copy link
Member

@@ -152,7 +151,7 @@ Composition.prototype.sequential = function() {
Composition.prototype.then = function(fulfill, reject) {
if (this.canceled) {
reject(new Error(COMPOSE_CANCELED));
return;
return this;
Copy link
Member

Choose a reason for hiding this comment

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

@tshemsedinov could you clarify if this should return this (to allow further chaining) or null that will result in 'null is not a function' if anyone tries to chain them further?

Copy link
Member

Choose a reason for hiding this comment

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

this is of for now, but we will use composition as thenable and with async/await so in future we will implement returning new composition from Composition.prototype.then

Copy link
Member

Choose a reason for hiding this comment

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

Need tests/examples and more research

@@ -26,12 +25,12 @@ const collect = (
const collector = (key, err, value) => {
if (isDone) return collector;
if (!isDistinct || !(key in data)) {
if (!isCount && !keys.has(key)) return;
if (!isCount && !keys.has(key)) return null;
Copy link
Member

Choose a reason for hiding this comment

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

return collector;

count++;
}
if (err) {
collector.finalize(err, data);
return;
return null;
Copy link
Member

Choose a reason for hiding this comment

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

return collector;

lib/chain.js Outdated
@@ -11,6 +11,7 @@ const async = op => {
case 'series': return series;
case 'find': return find;
}
return null;
Copy link
Member

@tshemsedinov tshemsedinov Oct 19, 2018

Choose a reason for hiding this comment

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

return (items, fn, callback) => {
  callback(null, items.map(id));
};

where
const id = x => x;

Copy link
Member

Choose a reason for hiding this comment

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

Why not simple
items.map(id) -> items.slice()?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, .slice(0) will be better

@@ -23,7 +24,7 @@ ArrayChain.prototype.execute = function(err) {

if (!item.op) {
if (err) throw err;
else return;
else return null;
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -37,7 +38,7 @@ ArrayChain.prototype.execute = function(err) {

if (err) {
this.execute(err);
return;
return null;
Copy link
Member

Choose a reason for hiding this comment

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

👍

if (Array.isArray(par)) {
while (par.length) {
const item = par.shift();
const request = delayed.shift();
if (request) request(item);
else items.push(item);
}
return;
return undefined;
Copy link
Member

Choose a reason for hiding this comment

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

return pool;

Copy link
Member Author

Choose a reason for hiding this comment

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

@tshemsedinov there is no pool available in this scope.

Copy link
Member

Choose a reason for hiding this comment

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

We need to put par => { to identifier pool and return it

@@ -33,6 +33,7 @@ const poolify = (factory, min, norm, max) => {
const callback = provide(par);
if (res) callback(res);
else delayed.push(callback);
return undefined;
Copy link
Member

Choose a reason for hiding this comment

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

return pool;

Copy link
Member Author

Choose a reason for hiding this comment

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

@tshemsedinov there is no pool available in this scope.

Copy link
Member

Choose a reason for hiding this comment

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

see above

if (par && par[poolified]) {
const delayed = pool.delayed.shift();
if (delayed) delayed(par);
else pool.items.push(par);
return;
return undefined;
Copy link
Member

Choose a reason for hiding this comment

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

return pool;

@@ -34,10 +34,11 @@ const poolify = (factory, min, norm, max) => {
const callback = provide(par);
if (res) callback(res);
else pool.delayed.push(callback);
return undefined;
Copy link
Member

Choose a reason for hiding this comment

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

return pool;

@@ -136,6 +136,7 @@ Queue.prototype.takeNext = function(
}
const task = tasks.shift();
if (task) this.next(task);
return this;
Copy link
Member

Choose a reason for hiding this comment

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

👍

@belochub
Copy link
Member Author

@lundibundi @nechaido @tshemsedinov ping.

@belochub belochub merged commit a5941eb into master Nov 1, 2018
@tshemsedinov tshemsedinov deleted the use-eslint-config-metarhia branch June 15, 2019 22:22
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.

4 participants