Code review comment for lp:~elbati/hr-timesheet/adding_hr_attendance_analysis_7

Revision history for this message
Lorenzo Battistini (elbati) wrote :

On 04/02/2013 02:37 PM, Guewen Baconnier @ Camptocamp wrote:
>
> * Why is there a dependency on python 2.7?

Because of timedelta.total_seconds()
<http://docs.python.org/2/library/datetime.html#datetime.timedelta.total_seconds>.

>
> l.146 from openerp.tools.translate instead of from tools.

Done

>
> l.162, what does means this 0.01666667 ?
> - the unit worth be precised in the `working_time_precision` field

How?

> - 1.0 / 60 is more readable than 0.0166...

Done

>
> l.221, typo in the precision I think

Done

>
> l.227 the timedelta can be far easier to read with keyword arguments (timedelta(hours=precision)

Done

>
> l.268 using \ is to avoid, even more in arithmetic operations I think, just use the continuation inside parenthesis if the line is too long
>
> minutes = (datetime.minute / 60.0 +
> datetime.second / 60.0 / 60.0)

Done

>
> l.1937-1983 using dicts would be appropriate here instead if many elif
>

Done

« Back to merge proposal