elm-test icon indicating copy to clipboard operation
elm-test copied to clipboard

Removing Fuzz.andThen

Open jwoudenberg opened this issue 8 years ago • 5 comments

Resolves #160 and #161.

This removes Fuzz.andThen making it a breaking change. It also fixes two points @mgold alluded to before:

  • the Fuzzer type is wrapped in a type instead of being a type alias to prevent people from casing on it. This is not a major change according to elm-package but it is to a user who for some reason is casing on the fuzzer now, so it makes sense to me to bundle the change in here.
  • Move the last two functions in Fuzz.Internal over to Fuzz.

Some things I think should be wrapped up before merging this:

  • [x] Make solutions for users using andThen for building recusive fuzzers or Msg fuzzers presentable, either by publishing them in a library or linking to them as a sort of cookbook.
  • [x] Consider if there's other breaking changes that should be bundled with this change. Personally, I'd like to change Test.Runner.fuzz so it doesn't need a Debug.crash anymore (the last in the codebase).

jwoudenberg avatar Jul 01 '17 12:07 jwoudenberg

  • I've removed the Debug.crash from Test.Runner.fuzz in favour of a Result.
  • I've published the Msg list fuzzer here: https://github.com/jwoudenberg/elm-test-experimental

From my perspective this is ready to merge but of course happy to make adjustments!

jwoudenberg avatar Jul 09 '17 09:07 jwoudenberg

Love it! Looks good to me!

rtfeldman avatar Jul 09 '17 14:07 rtfeldman

This looks good to me. I'm sorting through upcoming changes and don't want breaking API on master right this moment though.

mgold avatar Jul 09 '17 16:07 mgold

I'm cool with saying "this is ready to merge, but let's not push the button yet."

On Sun, Jul 9, 2017, 12:45 PM Max Goldstein [email protected] wrote:

This looks good to me. I'm sorting through upcoming changes and don't want breaking API on master right this moment though.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/elm-community/elm-test/pull/183#issuecomment-313931137, or mute the thread https://github.com/notifications/unsubscribe-auth/ABCxwH47tNyyGyct8Qh433GMbpYsYXgWks5sMQOtgaJpZM4OLTPO .

rtfeldman avatar Jul 09 '17 17:07 rtfeldman

Tagging major-release-blocker so we don't forget about this. Merge conflicts suck but I don't think it's worth resolving them now is we're still not ready to merge. Merging onto a breaking branch will just give us conflicts later.

mgold avatar Aug 01 '17 03:08 mgold