GTG

Code review comment for lp:~bertrand-rousseau/gtg/get_due_date

Revision history for this message
Izidor MatuĊĦov (izidor) wrote :

I reviewed your changes and I like the proposed behavior better. There goes nitpicking:

get_constrained_date() should be a private method (start with '_') or even better a private function (a defined function within function) because it isn't use anywehre but set_due_date() and it is not intended to be a public method

The correct code in editor should be:

if date_kind == GTGCalendar.DATE_KIND_DUE:
    date = self.task.get_due_date()

Otherwise it uses start date for the calendar - regression.

You've changed the description from "clear due date" into "reset to default". I am not sure if it is a good idea:
  1, it is inconsistent within due_date -> the button in calendar is still "clear"
  2, it is inconsistent with start_date -> why you clear start_date but reset due_date? In my opinion, it reveals too much implementation details to a user.

And as usually, the name of the program is intranslatible after using Glade :-/

It looks good, fix those small details and you have my aproval.

review: Needs Fixing (code, running)

« Back to merge proposal