search

M-Zuber / MyHome

Home finance management
MIT License
24 stars 21 forks source link

week cases added #119

Closed stoiandan closed 8 years ago

stoiandan commented 8 years ago

Added support for the week case.

Review on Reviewable

M-Zuber commented 8 years ago

Looks good. Would you like to finish this by making sure that both the RecurringExpense and RecurringIncome have all the needed code? Just make the changes and commit it to the branch. They will be automatically added to this PR, so there is no need to close and open a new PR

stoiandan commented 8 years ago

I also noticed that by mistake I added some blank space, sorry, got rid of it, and I rearranged the order in witch the method comes, that week follows after day and before month. The windows for both classes do have the same size so it should look decent.

M-Zuber commented 8 years ago

It seems you forgot to add the week option to the switch in the RecurringExpenseInput form. You can just copy it from the RecurringIncomeInput form

oh, and don't feel bad that you missed it. Thanks to this PR we caught a major bug that data wasn't being saved.

Otherwise the changes look great! Thank you for all the hard work.

stoiandan commented 8 years ago

Does it look good now?

M-Zuber commented 8 years ago

:shipit: :+1: Looks great!! Thank you very much!!