search

microadam / drone-config-changeset-conditional

The Unlicense
65 stars 17 forks source link

fix: support multiple pipelines #2

Closed Pajk closed 5 years ago

Pajk commented 5 years ago

The plugin was failing with a multi-pipeline yaml config when one of the pipelines were excluded.

I'm testing it with Drone 1.0-rc5 and other than this it works well so far. Thanks!

microadam commented 5 years ago

Hi @Pajk! apologies for the delay. I have only just seen this (need to fix my notifications!)

Thanks for spotting this. Could I just ask, what the .filter(pipeline => pipeline) is for? I believe there should always be a truthy value returned from the map function. However if that is not the case It would be great to see an example.

Ideally if you could possibly put some example YAML files in a comment that expose the issue, that would be great. I really need to get some unit tests in place (this was put together quite quickly) so that would allow me to cover this issue when I do.

Thanks for your input! Much appreciated

Pajk commented 5 years ago

You're correct .filter(pipeline => pipeline) doesn't have to be there anymore. I first tried to return null in case the pipeline was to be excluded but then I ended up with modifying the trigger.

Pajk commented 5 years ago

And without this patch it's failing in a case where you have more than one pipelines and one of them (except the last) has changeset trigger that doesn't match any changes.

microadam commented 5 years ago

Great! Your latest change makes perfect sense, I had thought about multiple pipelines but not properly tested them. Thanks for the fix :)

microadam commented 5 years ago

new version released to dockerhub with your fix :)