myntra / pipeline

Pipeline is a package to build multi-staged concurrent workflows with a centralized logging output.
MIT License
477 stars 54 forks source link

Pipeline.Run() should return a Result on both success and failure (instead of just failure) #2

Closed RobbieMcKinstry closed 7 years ago

RobbieMcKinstry commented 7 years ago

Hi! Thanks for Pipeline! I mentioned on your reddit post that this is definitely something I intend to use!

I'm trying to test a Step I implemented. I wrote the step, which simply shells out to the console and stores the stdout into the Result.Data field. My problem is that this isn't testable because after running the pipeline, a nil *Result is returned on success.

I propose pipeline.Run() always return a *Result on both success and failure. Simply return the *Result from the final Stage.

I'm going to fork this repo and implement the change for my own purposes. Let me know if you like the proposal and want me to PR back upstream. :)

Have a great day!

RobbieMcKinstry commented 7 years ago

The change was a single line, and it seems to work for me. I'll probably just PR anyway and you can close it if you don't like it.

adnaan commented 7 years ago

That sounds reasonable. Sure. Let me take a look at that PR.

adnaan commented 7 years ago

Merged the PR. Updated for side effects. Thanks again!

RobbieMcKinstry commented 7 years ago

@adnaan Sorry, I should have included these changed before I PR'd.

Thanks for the merge, friend!

https://github.com/myntra/pipeline/pull/4