Tryton Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(425)

Issue 46431002: account_asset: Allow to set when the depreciation move should be created

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 4 days ago by pokoli
Modified:
3 days, 14 hours ago
Reviewers:
ced, reviewbot
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 5

Patch Set 2 : Set depreciation moves as a company configuration #

Unified diffs Side-by-side diffs Delta from patch set Stats (+56 lines, -4 lines) Patch
M CHANGELOG View 1 chunk +2 lines, -0 lines 0 comments Download
M __init__.py View 1 1 chunk +1 line, -0 lines 0 comments Download
M account.py View 1 3 chunks +42 lines, -2 lines 0 comments Download
M asset.py View 1 2 chunks +7 lines, -2 lines 0 comments Download
M view/configuration_form.xml View 1 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 9
pokoli
1 week, 4 days ago (2018-05-09 15:15:26 UTC) #1
reviewbot
https://codereview.tryton.org/46431002/diff/1/asset.py#newcode49 asset.py:49: E711 comparison to None should be 'if cond is None:' https://codereview.tryton.org/46431002/diff/1/asset.py#newcode938 asset.py:938: E731 ...
1 week, 4 days ago (2018-05-09 15:15:39 UTC) #2
ced
https://codereview.tryton.org/46431002/diff/1/asset.py File asset.py (right): https://codereview.tryton.org/46431002/diff/1/asset.py#newcode369 asset.py:369: bymonthday=self.bymonthday or -1) For me, it should be: (self.bymonthday ...
1 week, 3 days ago (2018-05-10 16:00:54 UTC) #3
pokoli
Set depreciation moves as a company configuration
3 days, 16 hours ago (2018-05-17 14:28:16 UTC) #4
pokoli
https://codereview.tryton.org/46431002/diff/1/asset.py File asset.py (right): https://codereview.tryton.org/46431002/diff/1/asset.py#newcode369 asset.py:369: bymonthday=self.bymonthday or -1) On 2018/05/10 16:00:53, ced wrote: > ...
3 days, 16 hours ago (2018-05-17 14:28:50 UTC) #5
reviewbot
https://codereview.tryton.org/46431002/diff/20001/asset.py#newcode49 asset.py:49: E711 comparison to None should be 'if cond is None:' https://codereview.tryton.org/46431002/diff/20001/asset.py#newcode917 asset.py:917: E731 ...
3 days, 16 hours ago (2018-05-17 14:42:43 UTC) #6
ced
https://codereview.tryton.org/46431002/diff/1/asset.py File asset.py (right): https://codereview.tryton.org/46431002/diff/1/asset.py#newcode369 asset.py:369: bymonthday=self.bymonthday or -1) On 2018/05/17 14:28:50, pokoli wrote: > ...
3 days, 15 hours ago (2018-05-17 15:00:14 UTC) #7
pokoli
https://codereview.tryton.org/46431002/diff/1/asset.py File asset.py (right): https://codereview.tryton.org/46431002/diff/1/asset.py#newcode369 asset.py:369: bymonthday=self.bymonthday or -1) On 2018/05/17 15:00:14, ced wrote: > ...
3 days, 15 hours ago (2018-05-17 15:30:38 UTC) #8
ced
3 days, 14 hours ago (2018-05-17 15:59:04 UTC) #9
https://codereview.tryton.org/46431002/diff/1/asset.py
File asset.py (right):

https://codereview.tryton.org/46431002/diff/1/asset.py#newcode369
asset.py:369: bymonthday=self.bymonthday or -1)
On 2018/05/17 15:30:38, pokoli wrote:
> On 2018/05/17 15:00:14, ced wrote:
> > On 2018/05/17 14:28:50, pokoli wrote:
> > > On 2018/05/10 16:00:53, ced wrote:
> > > > For me, it should be:
> > > > 
> > > > (self.bymonthday or -1, -1)
> > > > 
> > > > to ensure to be last day of the month if the day does not exist on this
> > month.
> > > 
> > > This produces two moves when the month has the day, which I don't think
it's
> > the
> > > expected behaviour
> > 
> > But it needs to have a fallback if the bymonthday does not exist in the
month.
> 
> The problem is that this is not supported by rrule. See:
> 
>
https://stackoverflow.com/questions/38328313/dateutils-rrule-returns-dates-th...
> 
> And here is a PR with partial implementation:
> 
> https://github.com/dateutil/dateutil/pull/522

Then I think we should forbid to use numbers greater than 28.
Or just limit to 1 or -1.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld bc8b922