thoughtbot / carnival

An unobtrusive, developer-friendly way to add comments
MIT License
499 stars 30 forks source link

Better test module organization, etc #150

Closed pbrisbin closed 9 years ago

pbrisbin commented 9 years ago

This removes the need for Helpers.Assertions

This (the rename and function move) gets us closer to the default scaffold.

This replaces Helpers.DB. All functions live in DB for composability and functions used by only one Example were moved to be directly with it.

This will soon become a PR to upstream

They've never been used and only increase maintenance burden. In my next PR, I'll remove the Handlers themselves (and any code they use).

It is only used there.

jferris commented 9 years ago

Remove tests for unused-Handlers They've never been used and only increase maintenance burden. In my next PR, I'll remove the Handlers themselves (and any code they use).

I'd recommend removing those handlers and their tests in a separate pull request on master and then rebasing this on top of the branch for that pull request.

pbrisbin commented 9 years ago

I'd recommend removing those handlers and their tests in a separate pull request on master

FYI: #151

pbrisbin commented 9 years ago

Build is failing due to this: https://github.com/mietek/halcyon/issues/36

mietek commented 9 years ago

On it.

mietek commented 9 years ago

https://github.com/mietek/halcyon/issues/36 should now be fixed. The fix is now tested and merged to master.

Please try again, keeping in mind if you’ve disabled Halcyon self-updates, you’ll need to re-enable them, or update Halcyon manually.

pbrisbin commented 9 years ago

@jferris rebased, green build, ready for a final look.

pbrisbin commented 9 years ago

Oh, and thanks for the quick fix @mietek !

jferris commented 9 years ago

Looks good to merge.