[PATCH] ChangesetCommentsModel: refactor duplicate code

Jan Heylen heyleke at gmail.com
Thu Apr 30 01:07:25 EDT 2015


On Thu, Apr 30, 2015 at 12:44 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 04/29/2015 10:09 PM, Jan Heylen wrote:
>>
>> -
>> -        q = ChangesetComment.query()\
>> -                .filter(ChangesetComment.repo_id == repo_id)\
>> -                .filter(ChangesetComment.line_no == None)\
>> -                .filter(ChangesetComment.f_path == None)
>> -        if revision:
>> -            q = q.filter(ChangesetComment.revision == revision)
>> -        elif pull_request:
>
>
> btw: it seems like this chunk of code (that you are de-duplicating but not
> directly touching) could misbehave if revision or pull_request somehow was
> specified as something that was non-null but had false value. It should thus
> use 'if revision is not None' instead (and perhaps assert pull_request is
> None).

Ok, I'll have a look at that, in a separate commit, as a refactor
shouldn't touch code too much in my belief, mostly reorganise and
move...Dangerous for code reviewers that see "refactor", but actually
the "refactor" contains "fixes" too...

Jan

>
> /Mads


More information about the kallithea-general mailing list