Set Money.default_currency to nil. #868

thealiilman · 2019-05-21T17:56:01Z

This PR closes #855.

coveralls · 2019-05-22T09:52:50Z

Coverage remained the same at 100.0% when pulling 757249b on thealiilman:enhancement/855-made-default-currency-to-be-nil into 3f9ecc2 on RubyMoney:v7_new.

spec/money_spec.rb

antstorm · 2019-05-25T10:51:39Z

spec/money_spec.rb

@@ -181,29 +181,35 @@
    end

    it "accepts an optional currency" do


You should probably split these tests into context — with a default_currency set and with it being nil, because the behaviour is different. From what I can tell right now a case when a default_currency is nil and you initialize it without a currency is untested.

You should probably split these tests into context — with a default_currency set and with it being nil, because the behaviour is different. From what I can tell right now a case when a default_currency is nil and you initialize it without a currency is untested.

these tests being, for Money.from_amount or for others such as Money#amount etc? 🤓
I have added two more contexts in .from_amount's block.

lib/money/currency.rb

antstorm · 2019-05-25T10:56:59Z

@thealiilman thanks for looking at this! I've left some comments

thealiilman · 2019-05-26T09:57:13Z

Thanks for the feedback @antstorm! I've pushed the changes based on your comments, also left a question regarding splitting the tests into contexts. 🙂

spec/money_spec.rb

antstorm · 2019-06-01T13:08:45Z

Looks great, thank you! 👍

thealiilman force-pushed the enhancement/855-made-default-currency-to-be-nil branch from 39c3bba to bfc017e Compare May 21, 2019 17:57

thealiilman changed the title ~~Enhanced default_currency to force developer(s) to set its value.~~ Made default_currency to be nil. May 21, 2019

thealiilman force-pushed the enhancement/855-made-default-currency-to-be-nil branch 2 times, most recently from 15b015e to 633f0b1 Compare May 22, 2019 09:38

thealiilman changed the title ~~Made default_currency to be nil.~~ Set Money.default_currency to be nil. May 22, 2019

thealiilman marked this pull request as ready for review May 22, 2019 09:44

thealiilman changed the title ~~Set Money.default_currency to be nil.~~ Set Money.default_currency to nil. May 22, 2019

thealiilman force-pushed the enhancement/855-made-default-currency-to-be-nil branch from 633f0b1 to bd2b050 Compare May 22, 2019 09:48