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

Adding public information about CI status of the library #848

Closed
elhigu opened this issue Feb 15, 2018 · 7 comments
Closed

Adding public information about CI status of the library #848

elhigu opened this issue Feb 15, 2018 · 7 comments
Labels

Comments

@elhigu
Copy link

elhigu commented Feb 15, 2018

I could write Travis CI configuration which runs driver tests with node 4,6,8 and 9 against Oracle Express g11 and add build status badge. Would you like PR for that?

@cjbj
Copy link
Member

cjbj commented Feb 15, 2018

@octokyle runs all those tests (and many more, eg. with different Oracle Clients and DBs) during our release cycle so I think you can save time and work on other enhancements! But the offer is appreciated.

@cjbj cjbj added the question label Feb 15, 2018
@elhigu
Copy link
Author

elhigu commented Feb 15, 2018

Well CI should have catched for example #847 already before deploy process when error landed to master (well maybe would have, I don't know if that part was covered by tests suite).

Automatic testing is usually more robust and catches them earlier than running tests manually. Of course CI doesn't replace separate deploy testing completely, since CI cannot test against all oracle db versions etc.

Offer is open though. I don't think I want to be involved with other enchantments though, I have already my hands full with knex maintenance :) That travis CI setup would just have been pretty easy to copy-paste from my other repos.

@cjbj
Copy link
Member

cjbj commented Feb 16, 2018

@elhigu let's not mention #847 until we know what that happened there !!!

Have you got a CI solution that satisfies all Oracle license click-through requirements?

@elhigu
Copy link
Author

elhigu commented Feb 16, 2018

@cjbj sure, lets not mention that :) I actually verified that our approach was fine with people from local oracle representation which they confirmed that it is all good.

It sounds pretty reasonable that they actually want to enable use of oracle dbs to be more accessible for people in node community (we would need to drop the support right away if we couldn't run CI on it).

@cjbj
Copy link
Member

cjbj commented Feb 16, 2018

What's valid for you may not be for me - or may not be easy. Excuse me getting grumpy at the thought having to sort out legals and business stuff. Particularly for things that seem no-brainers to mere mortals like us.

@elhigu
Copy link
Author

elhigu commented Feb 18, 2018

I see you are worried about it making life harder if this feature would be merged and that sounds perfectly good reason to me.

Would you like PR for that?

tl;dr answer was no :)

@elhigu elhigu closed this as completed Feb 18, 2018
@cjbj
Copy link
Member

cjbj commented Feb 20, 2018

@elhigu thanks for understanding. Since we push commits to an internal repo first, the best place to do CI is there, and not duplicate effort etc on GitHub. There are more than enough other projects to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants