oadam / proprio

A free property management software to manage your tenancies
MIT License
20 stars 13 forks source link

Enabled partial month tenancy, made german translations #21

Closed vectorien closed 8 years ago

vectorien commented 8 years ago

Hi I have done some work in order to enable the possibility to rent something for some days only, or start renting at a specific date, so the field validation to enter the first of month date is obsolete. I hope this is not against your development goals, but I find it makes sense. Cheers

oadam commented 8 years ago

Hi,

I made a couple of remarks. The most important one is that I would prefer that we don't use tenancy.start_date and tenancy.end_date to generates rents. Instead I would prefer to use start_date and end_date from RentRevision.

Also next time can you try to split your commit in smaller pull requests ?

Apart from that the change looks good, I'm looking forward to merging it.

Thanks, Olivier

vectorien commented 8 years ago

yeah sorry. at the time I thought about a pr, it was already so big next time will be smaller. Maybe for example the test case in a later pr.

As for the mentioned variables, I commented further up

oadam commented 8 years ago

Hi @vectorien,

I've cherry-picked some of your suggestions to the main branch. I did not have time to fully review the "meat" of the change (partial rents) due to lack of time but I've noticed that it broke some of the automated tests. I've reconfigured the repository so that it uses travis-ci, so hopefully broken tests will stand out more in future pull requests. I think the change could be smaller (and do not break tests :smile: ). My recommended approach is to :

If you have any question, please feel free to chat me during the day on hangout (my gmail is in the commit messages) or skype (skype id: adam_olivier)

Thank you again and cheers !

vectorien commented 8 years ago

Ok then lets close this pr and I implement the things u mentioned , then i'll make a new pr. cool?

oadam commented 8 years ago

Sure. Thanks ! Le 19 nov. 2015 6:22 PM, "vectorien" notifications@github.com a écrit :

Closed #21 https://github.com/oadam/proprio/pull/21.

— Reply to this email directly or view it on GitHub https://github.com/oadam/proprio/pull/21#event-469430533.