Build with NaN

[Bigous]

Hi people,

This pull intend to solve issue #2 .

After some headaches, I could compile and run this driver wrapper with the NaN on IOJS 1.5.0 x64 on Windows.

I've tested only select commands and I need help to test everything, but it's usable.

We need some automated tests to see if all of this is working properly.

People who wanna run on IOJS on Windows, must run with iojs.exe and not node.exe due to a bug in iojs.

Hope have helped.
tks.
Richard

[cjbj]

Fantastic! It would be great if you can sign the OCA (see CONTRIBUTING.md) so we can merge this.
Releasing a test suite is still backed up for non technical reasons out of my control.

[Bigous]

Hi @cjbj

I've sent it a cuple of minutes ago.

Tks for your fast reply! ;)

[alexsantos]

Just compiled it on a OL6.6 x86_64 and it worked. Just some warnings on the examples.

[node@nodeserver oracle]$ iojs webapp.js 
sys is deprecated. Use util instead.
util.puts: Use console.log instead

[Bigous]

Hi @alexsantos ,

The warning was in the examples and not in the driver wrapper itself.

I removed the sys dependency from webapp.js, just to be clean ;)

Tks for your help.

[alexsantos]

👍

[Bigous]

I have merged the latest commits to this pull so it can flawlessly be merged.
Please, consider to merge it soon so we could avoid merge issues.

[lebolo]

👍

[alexsantos]

👍

[cjbj]

Hi @Bigous any merge will be some time off since the devs are busy continuing the other features (and projects) that they had already started and will need time to evaluate your patch. I was told your OCA form should be being processed this week. There are also some other administrative things pending, too. I'll let you know the progress.

[cjbj]

@Bigous can you add the signoff on your commit? See https://github.com/oracle/node-oracledb/blob/master/CONTRIBUTING.md

[pkoretic]

👍
tested this on Ubuntu 14.10 with node 0.10, 0.12 and on Arch Linux with iojs 1.6.1.
It all works as expected in our tests

It would be nice to have it merged soon @Bigous @cjbj :)

[cjbj]

@pkoretic thanks for the tests!

[Bigous]

@cjbj Merged to be easier to accept the pull.

[pkoretic]

@cjbj any news on this one? :)

[cjbj]

@pkoretic our internal approval process is progressing, but is not complete. I think the biggest hurdles have been jumped (famous last words!)

[Bigous]

Hi @cjbj ,
Any news on this one?

[cjbj]

@Bigous approvals are still pending. I did some door knocking today to help nudge the issue along.

[Bigous]

Ok, tks Chris

[hertzg]

Any updates?

[cjbj]

@hertzg waiting on just one final approval.

[hertzg]

@cjbj Great! thx for update

[Bigous]

@cjbj , I'm merging my fork with version 0.5 and got some warnings. almost all of them because DPI_SZ_TYPE to unsigned long when calling GetOutBinArray with executeBaton->rowsAffected...
I'll post my merge, but it could be a problem (in original code).

[cjbj]

@Bigous I'll pass that on. Sorry we are still in a holding-pattern on official NAN support.

[fuson]

@Bigous, i can't build your fork with this PR on Mac OS X (Node 12.2):

CXX(target) Release/obj.target/oracledb/src/njs/src/njsConnection.o
../src/njs/src/njsConnection.cpp:1254:49: error: conditional expression is ambiguous; 'Localv8::Primitive' can be converted to
'Localv8::String' and vice versa
( binds->ind[index] == -1 ) ? NanNull() :
^ ~~~~~~~~~
../src/njs/src/njsConnection.cpp:1261:48: error: conditional expression is ambiguous; 'Localv8::Primitive' can be converted to
'Localv8::Integer' and vice versa
(binds->ind[index] == -1 ) ? NanNull() :
^ ~~~~~~~~~
../src/njs/src/njsConnection.cpp:1266:48: error: conditional expression is ambiguous; 'Localv8::Primitive' can be converted to
'Localv8::Number' and vice versa
(binds->ind[index] == -1 ) ? NanNull() :
^ ~~~~~~~~~

[Bigous]

Hi @fuson, I have been trying to keep the code as close to the original as possible, but, to solve this ambiguity, I had to change a little bit the code to avoid operator ?: together with NanNull().

I dont have a Mac to test. Could you see if it works for you?

tks.

[cjbj]

Yay! Thanks @Bigous. There are still a few compile warnings on some versions of Linux we'll look at.

[hertzg]

Thanks @Bigous & @cjbj

[cjbj]

@hertzg All credit is to @Bigous

@richlim I'm tagging you here since you explicitly mentioned NAN in #85. I hope you will be able to contribute to node-oracledb.

[Bigous]

Hi @cjbj Thanks - you had a lot of effort to merge this one ;) almost a novel!