Bithound.com and Travis-Ci

[agershun]

@mathiasrw You just added a the special config file for http://bithound.com.

Do we need something similar for https://travis-ci.org/ ?

Unfortunately, I have never worked with these services. Will learn it.

[agershun]

Important note: We need to check only two files: alasql.js and alasql-worker.js (plus all plugins)

[mathiasrw]

Bithound is a service to help point out where we can make the code better. Its the first graphic in the readme.

As it comes with specifik suggestions linking to specifik lines or functions I imagine its better to let it look in the files in src to help spot the exact location.

If we make it to look in the compiled alasql.js you will have to manually locate the specific line in one of the files in src.

Do you want me to change it to look into the compiled version?

[mathiasrw]

At first I was gonna connect Travis too, but putt it off to later.

Ideally we must configure it to compile from src with gulp and run the tests on the compiled alasql.js.

[agershun]

I would prefer alasql.js. The reason - every file is just a slice of alasql.js, and not separate JavaScript entity. I understand, that is harder to search errors by line numbers, but... let's do it so. Thank you!

[mathiasrw]

👍

It will now look into all js in dist that is not *min.js

The new changes only take effect when you merge the .bithoundrc file into the master branch...

[mathiasrw]

Hmm. I make the bithound work on the dev branch.

Pfffgghhhh - looks like the alasql.js is too big to be analyzed... hmmm... looking into it

[mathiasrw]

Agfhhhph

bitHound will blacklist a file if it is larger than 100KB. Files of these size are usually artifacts of a build process, and should be ignored using .bithoundrc.

If this, however, is a project file that work normally gets committed to, you should consider splitting it into smaller, more maintainable modules.

We have to change it back to src

[agershun]

... it is better if we follow this recomendatiion:

splitting it into smaller, more maintainable modules.

Let's do it on this way:

1. Now - return the setup back
2. After we will split it in more logical parts

[mathiasrw]

The logical parts make sense if / when we implement the modular setup - so one can parser and backend (well - the last part you can)

But there is also a force in having a "THIS IS THE ONE FILE YOU NEED" file for people to get started.

Still thinking...

[agershun]

@mathiasrw I am a little bit tired and want to take couple days off from alasql.js source code.

If you have time and wish, you can fix these bithound problems like ; and others. I will not touch the code for these days, and after we will take you version and prepare release 0.1.10 with improved quality of code. At the same time, we will check and ajdust "How to accemble alasql.js" manual.

The code in debug branch is now updated ( or uptodated :) ) .

It will be great, if at the same time you can document your changes with jsDoc tags (we will use it for API documentation and Closure Compiler).

I will spend this time to finish Codex and prepare some documentations for users.

Thank you!

[mathiasrw]

Thanks for letting me know. You have been a machine providing code AND responded to everything within minutes - so makes good sense you need some fresh air.

I will make a git-flow feature branch - so we stick to the idea of git-flow - that way we should never have problems with unupdated code :)

I will put jsDoc tags where it makes sense - but most of the bithound things are tiny semantic elements.

[mathiasrw]

Note to self about JSlint

/*jslint evil: true */
var add = new Function("a", "b", "return a + b");

JSLint found no errors

[agershun]

Eval is definitely evil. That is why AlaSQL is so fast :) By the way, CrossFilter also uses this approach.

[mathiasrw]

I guess the reason they warn is that most people dont get when to use and when not to use this feature.

Was crossfilter where you got your inspiration from?

I really like that the cashing of AlaSQL is based on a custom built functions - it feels sneaky :)

[agershun]

\Yes and no. The source of inspiration were:

• My current budgeting software project (we have a lot sorting/grouping/etc.)
• Lodash
• SequelSphere
• Jison + SQL Parser.
• Plus, thanks to Mozilla and their decision not to support WebSQL.

This is the first post where AlaSQL started: lodash/lodash#479 (This is when the history begins!)

[mathiasrw]

👍

[mathiasrw]

Question: Shuld I be worried about the comment here https://github.com/agershun/alasql/blob/1706fdcb2b5166490d8e2492d53b2a14a5c81d21/src/17alasql.js#L247

adrunone(); /** @todo Check, why data is empty here */

[mathiasrw]

What is the point of

 (ñ) 

in 05copyright.js line 7?

[mathiasrw]

In what file is the end of the UMD envelope placed?

(start is here https://github.com/agershun/alasql/blob/1706fdcb2b5166490d8e2492d53b2a14a5c81d21/src/10start.js#L25 )

[mathiasrw]

I remember you told me that you went from .async to .promise for async execution

in the jsDoc comments I found this example

alasql('SELECT * FROM ?',[data],function(res){
    console.log(data);
});

is this still supported? is .promise the same?

[agershun]

No, the promised version is here:

    alasql.promise('SELECT * FROM ?,[data])
     .then(function(data) {
            console.log(data);
      }).catch(function(err) {
            console.log(err);
      });

I just changed the name of the function to more clear from async() to ``promise()`.

The async varian is still supported:

    alasql('SELECT * FROM ?',[data],function(res){
        console.log(data);
    });

as well as sync:

    var res = alasql('SELECT * FROM ?',[data]);

[agershun]

1. There is a definitely inconsistency in these two lines. Unfortunately, I can not remember why I left data parameter blank. Please, leave this @todo comment, we will check it later.

    adrunone(); /** @todo Check, why data is empty here */

    function adrunone(data) {

1. (f) May be this is transformation of copyright sign? :)) Can you fix this line and add your name?
2. End of UMD is in file 98finish.js

[mathiasrw]

👍

Found a merge error in 72delete.js. Its fixed and placed in develop, but you might want pull the change.

[agershun]

Just pulled and pushed. Ok.

[mathiasrw]

Can you describe the problem you mention here https://github.com/agershun/alasql/blob/c0745fc0fb6b257d3771fc93f22d489ccc295daa/src/38query.js#L405 ?

// TODO: Problem with DISTINCT on objects

Can we make a test that verify if the problem is still there?

[mathiasrw]

I dont get the syntax for

ms:Date.now()-ms

in https://github.com/agershun/alasql/blob/c0745fc0fb6b257d3771fc93f22d489ccc295daa/src/38query.js#L277

can you put a word to how it works?

[agershun]

Question 1

// TODO: Problem with DISTINCT on objects

Current loop below this message does not do comparision of nested objects. It is ok for plain SQL, but not ok, if we going to query JSON objects.

We need to review all distinct() functions from 15utility.js, keep one which is:

• fast
• does proper deep (nested) check for distinct objects

Put it here and in other places with DISTINCTs.

Question 2
Syntax for:

ms:Date.now()-ms

I wanted to create something like EXPLAIN keyword. Here Date,now() returns current time in milliseconds. ms is variable from here. So the result is number milliseconds spent for sorting.

[mathiasrw]

Can you elaborate on the relation between the identical code in

• https://github.com/agershun/alasql/blob/2277cc08e5210be48206fef992e7c6dd22b42fd6/src/50expression.js#L787-L798

and

• https://github.com/agershun/alasql/blob/2277cc08e5210be48206fef992e7c6dd22b42fd6/src/60createtable.js#L10-L21

[mathiasrw]

Nope - never... but looks really awesome !

[mathiasrw]

Can you elaborate on https://github.com/agershun/alasql/blob/2277cc08e5210be48206fef992e7c6dd22b42fd6/src/50expression.js#L112

return returnTrue()

with the definition

function returnTrue () {return true;}

must be the same as return true in the first statement. Is it because we cant to return the function returnTrue to be executed later and should return the function name without invoking it?

[agershun]

According the initial idea it should be

    return returnTrue;

I do not know, whey I left the construction with parenthsis, may be this is a bug. Will discover it tonight. Thank you!

[mathiasrw]

Hah - ok. Sorry for poking the code so much and asking all sorts of questions - but guess it's worth it if we get rid of some bugs on the way :)

By the way - check out this - it's important for our MySQL compliant flavour

http://mysqlserverteam.com/json-labs-release-native-json-data-type-and-binary-format/

[mathiasrw]

Some more details on the mysql JSON date

http://mysqlserverteam.com/json-labs-release-json-functions-part-1-manipulation-json-data/

[agershun]

As far as I understand it is not so hard to realize MySQL and Prostres JSON functions and casting procedures. It wlll take day pr two. BTW IMHO original AlaSQL's JSON is better than MySQL/Postgres/SQL Server realizations. Ok, I will create issue and some code :)

[mathiasrw]

I agree - its better and more "native" (as its truely native to Javascript)

But we still need to support the syntax if we want to make a flavour thats 100% mysql

Question

Are you using the functions in 12pretty.js for debugging during development? Calls to functions like L, N, NL, ID, KK are spread out in the code a lot of places, and I dont quite get the point

[agershun]

1. AlaSQL uses .toString() to reconstruct names for expressions to create expression column names. For example:

    SELECT a+b FROM @[{a:1,b:2}]

generates:

    [{"a+b":3}]

Here "a+b" was reconstructed from AST with .toString() function. It is better than uses original sql text, because people can add spaces, line break, etc. And this is important for GROUP BY and ORDER BY clauses.

1. I had the idea to have prettifyer inside AlaSQL, But, probably, this function is not demanded.

So, we can:

1. Remove (or move to plugin) all .toString() functions for all non-expressions.
2. Remove all K(), L(), etc. calls and functions from .toString()

[agershun]

bad eval() flag. Is it possible to turn off this flag, because AlaSQL uses eval()(new Function()) knowingly by design?

[mathiasrw]

Sure. Its added now...

[mathiasrw]

Hey - can you elaborate on https://github.com/agershun/alasql/blob/develop/src/41exists.js#L18-22
?

How come both tableid and defcols are never used? Is it a never implemented idea? is it copy paste shadows?

[agershun]

Good question. All AlaSQL .toJavaScdipt() functions have these standard parameters.

yy.ExistsValue.prototype.toJavaScript = function(context,tableid,defcols) {

The question is:

• should we remove them in each particular case, where we do not use them or keep the same for all toJavaScirpt() for consitency?

From quality point of view it is better to keep the same headers with all parameters.
From size reduction - better to omit them.

The compromise may be to put unnecessary parameters into comments:

yy.ExistsValue.prototype.toJavaScript = function(context /*,tableid,defcols */) {

[agershun]

I found two other services (for future):

• https://codeclimate.com - for complex project evaluation
• http://inch-ci.org/ - for documentation evaluation

It is interesting, who can evaluate AlaSQL wiki? :)

[agershun]

Couple more badges for our README.md

• https://codeclimate.com/github/agershun/alasql

[mathiasrw]

should we remove them in each particular case, where we do not use them or keep the same for all toJavaScirpt() for consitency?

Hmmm. Its an issue while developing - not for runtime. Would still be slick if we could have a alasql version for development having a check on the values throwing an error if used

//IF_DEV
tableid||defcols ? throw "Wrong use": ;
//IF_DEV

A check that could be stripped out for the normal production version. Is it crazy?

[mathiasrw]

Could also be a comment in a format where if compiled specific to dev it would get into the code. Like

/*IF_DEV
tableid||defcols ? throw "Wrong use": ;
IF_DEV*/

where a "get dev version" would simply strip the source from the strings /*IF_DEV and IF_DEV*/

[mathiasrw]

Hmm. The last input would also make it possible to have

yy.ExistsValue.prototype.toJavaScript = function(context) {

in production but having

    yy.ExistsValue.prototype.toJavaScript = function(context /*IF_DEV , tableid, defcols IF_DEV*/) {

in src

[agershun]

Let's simplify and remove unnecessary parameters. We have few standard functions like this, and probably it is better to describe them at API level.

[mathiasrw]

👍

[mathiasrw]

I am looking into codeclimate.com - for complex project evaluation

http://inch-ci.org is buggy - but Im in dialogue with them about why

[agershun]

Cool!

Отправлено с iPhone

9 июня 2015 г., в 20:26, Mathias Rangel Wulff notifications@github.com написал(а):

I am looking into codeclimate.com - for complex project evaluation

http://inch-ci.org is buggy - but Im in dialogue with them about why

—
Reply to this email directly or view it on GitHub.

[mathiasrw]

In

https://github.com/agershun/alasql/blob/master/src/50expression.js#L276

and

https://github.com/agershun/alasql/blob/master/src/50expression.js#L281

The core of the return statement is

 (a).valueOf()===(b.valueOf())

For consistency I have changed it to

 (a).valueOf()===(b).valueOf()

Please let me know if the original structure was intentional...

[mathiasrw]

Please confirm that

https://github.com/agershun/alasql/blob/master/src/63createvertex.js#L222-L223

is only for debugging

[mathiasrw]

Is it an error that the two lines both contain !tbid? is first one ment to be overwritten?

https://github.com/agershun/alasql/blob/master/src/424select.js#L118-L119

[mathiasrw]

thtd is not defined. Can you please review

https://github.com/agershun/alasql/blob/master/src/424select.js#L195

[mathiasrw]

in 84from.js I found this line

if(res[i] == +res[i]) res[i] = +res[i];

I dont really get it. Can you elaborate on the idea?

[mathiasrw]

Regarding codeclimate.com - we have a new config file in develop so hopefully next time we deploy a version it will work...

[mathiasrw]

I will move the questions to one issue each