jklmli / monapt

Options, Tries, and Futures for JavaScript / TypeScript
MIT License
172 stars 13 forks source link

fix(try): pass error into matcher #16

Closed mrmurphy closed 7 years ago

mrmurphy commented 7 years ago

fix(try): pass error into matcher

Pass a Try's error into the Failed case on a match.

Closes https://github.com/jiaweihli/monapt/issues/15.

jklmli commented 7 years ago

Looks good - I missed this during the 1.0 migration. I'll also add some tests for this later.

Can you format your commit message in conventional style? e.g. fix(try): pass error into matcher. I'm trying out another tool, semantic-release, that automatically parses commit messages and generates a changelog/release/npm version. It'd be neat to see if it works out end-to-end.

mrmurphy commented 7 years ago

Sure thing!

jklmli commented 7 years ago

Sorry, that was unclear. I meant the commit itself (via git commit --amend, then update the PR with git push -f origin) rather than the PR description.

Also, can you remove the BREAKING CHANGE? That'll unfortunately cause a major version bump to 2.0.0.

If this is too much trouble, let me know. The last thing I want is for PRs to be tedious. I'm working on integrating tooling to help automate all of this.

mrmurphy commented 7 years ago

Oh okay! No that'll be fine 😊. I'll just have to do it late tonight or tomorrow when I'm not only on my phone.

jklmli commented 7 years ago

No problem - thanks for working through this with me. 👍

Happy to hear you're getting value out of monapt!

mrmurphy commented 7 years ago

No problem! Thanks for being quick to respond. I've updated the commit message now and taken out the BREAKING CHANGE line.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 98.81% when pulling dff5b6150bfa99579d502e712b10644644ac0ac2 on bloom:master into ec12411367aace75eb37c0af779ae5e0eb6a5fed on jiaweihli:master.

jklmli commented 7 years ago

Perfect, landed on npm and released with a changelog.

mrmurphy commented 7 years ago

Also, sorry I didn't add a test for this. I'd like to be better at that in the future. 😅

jklmli commented 7 years ago

No worries - tests are very much appreciated but I know they're time-consuming to write.

Looks like I need to tweak the code coverage though... 😛