qiita-spots / qp-deblur

2 stars 7 forks source link

Fixing param order #8

Closed josenavas closed 7 years ago

josenavas commented 7 years ago

Last PR, this fixes the last bit of the plugin so it works with Qiita.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 93.96% when pulling 21caf6bdc6ba9f471c716a4a7a4484671484e256 on josenavas:fix-empty into a84d52169dd0a0f161b7f7ac2f44ae0ea6cd3a8b on qiita-spots:master.

mortonjt commented 7 years ago

This seems like just a silly mistake - do you think it warrants a unittest?

If not - I think it is good to go.

josenavas commented 7 years ago

The problem that I have with this is that I don't know how to generate a good test case that triggers this error. I'm testing this in the test environment using a specific dataset that I think would be to big to add to the repo for testing.

mortonjt commented 7 years ago

Ok. For this given case I wouldn't worry about it given that this is pretty easy to catch. I just wonder how we missed this ...

mortonjt commented 7 years ago

@antgonza want to merge this?

antgonza commented 7 years ago

@mortonjt, nothing is perfect and sometime this happens, even after N reviews. Anyway, merging.