search

Overdrivr / micro-ci

Continuous Integration for embedded platforms
0 stars 0 forks source link

Jenkins #31

Closed DanFaudemer closed 8 years ago

DanFaudemer commented 8 years ago

Create an overlayer for jenkins library

Overdrivr commented 8 years ago

Nice job Dan ! Here is my feedback

  1. Could you rename lib/jenkins.js file to a different name to avoid potential conflicts with the jenkins module ?
  2. Use everywhere the return-early strategy I detailed in the line comments
  3. Fix indentation which seems broken in some places
  4. No need to have this.something in the tests, rather it is ok to have global objects

Cheers !

DanFaudemer commented 8 years ago

Hi,

Thanks Remi for all these feedbacks. I will implement them.

For your question: Bind: bind is used when you are using callback in a class. If you do not bind the function 'this' will be lost. And I cannot call method class.

DanFaudemer commented 8 years ago

One question: It is not mandatory to return all the expected parameters for a callback (as you mentioned above in case of error) ?

DanFaudemer commented 8 years ago

Forgot to answer of one of your question buildLog is used in my test. See GetLog

DanFaudemer commented 8 years ago

Hi Remi,

I think I have implemented all of your feedback. Except the one for the async. I also add create_node and remove_node function. Let me know if it is ok for you.

Dan

Overdrivr commented 8 years ago

One question: It is not mandatory to return all the expected parameters for a callback (as you mentioned above in case of error) ?

No, missing parameters will be replaced by null or undefined. So if you have three parameters and only care about the first, simply omit the others

Overdrivr commented 8 years ago

It should be good now ;)

DanFaudemer commented 8 years ago

:+1: