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

Issue 62001: First version of pysql

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 4 months ago by cugg
Modified:
10 months, 1 week ago
Reviewers:
cugg
Visibility:
Public.

Patch Set 1 #

Total comments: 28
Unified diffs Side-by-side diffs Delta from patch set Stats (+199 lines, -0 lines) Patch
A qm.py View 1 chunk +199 lines, -0 lines 28 comments Download

Messages

Total messages: 6
nicoe
12 years, 4 months ago (2011-07-13 08:33:13 UTC) #1
bch
http://codereview.tryton.org/62001/diff/1/qm.py File qm.py (right): http://codereview.tryton.org/62001/diff/1/qm.py#newcode26 qm.py:26: if self.where!=[]: Because there is no default value and ...
12 years, 4 months ago (2011-07-13 09:05:43 UTC) #2
nicoe
http://codereview.tryton.org/62001/diff/1/qm.py File qm.py (right): http://codereview.tryton.org/62001/diff/1/qm.py#newcode20 qm.py:20: cpt=0 spaces around '=' (and '>' and '+=', etc ...
12 years, 4 months ago (2011-07-13 09:12:28 UTC) #3
yangoon
12 years, 4 months ago (2011-07-13 09:16:25 UTC) #4
udono2
12 years, 4 months ago (2011-07-13 09:43:48 UTC) #5
cugg
12 years, 4 months ago (2011-07-14 06:17:36 UTC) #6
http://codereview.tryton.org/62001/diff/1/qm.py
File qm.py (right):

http://codereview.tryton.org/62001/diff/1/qm.py#newcode20
qm.py:20: cpt=0
On 2011/07/13 09:12:28, nicoe wrote:
> spaces around '=' (and '>' and '+=', etc (but not inside function calls and
> definition)).
> 
> See PEP8.

Done.

http://codereview.tryton.org/62001/diff/1/qm.py#newcode21
qm.py:21: for expression in self.expressions:
On 2011/07/13 09:12:28, nicoe wrote:
> ", ".join(self.expressions) is better

Done.

http://codereview.tryton.org/62001/diff/1/qm.py#newcode24
qm.py:24: expressions_to_print+=expression.__str__()
On 2011/07/13 09:12:28, nicoe wrote:
> expression.__str__() == str(expression)

Done.

http://codereview.tryton.org/62001/diff/1/qm.py#newcode26
qm.py:26: if self.where!=[]:
On 2011/07/13 09:05:43, bch wrote:
> Because there is no default value and there is no check that the 'where'
> attribute is present, this line raise:
>  AttributeError: 'Select' object has no attribute 'where'
> 
> When no 'where' as been given.

Done.

http://codereview.tryton.org/62001/diff/1/qm.py#newcode26
qm.py:26: if self.where!=[]:
On 2011/07/13 09:12:28, nicoe wrote:
> if self.where

Done.

http://codereview.tryton.org/62001/diff/1/qm.py#newcode36
qm.py:36: if self.group_by!="":
On 2011/07/13 09:12:28, nicoe wrote:
> "if self.group_by" 
> 
> is better

Done.

http://codereview.tryton.org/62001/diff/1/qm.py#newcode43
qm.py:43: if cpt_params!=0:
On 2011/07/13 09:12:28, nicoe wrote:
> "if cpt_params"
> 
> is better

Done.

http://codereview.tryton.org/62001/diff/1/qm.py#newcode96
qm.py:96: return value
On 2011/07/13 09:12:28, nicoe wrote:
> could return directly

Done.

http://codereview.tryton.org/62001/diff/1/qm.py#newcode121
qm.py:121: for key in keys:
On 2011/07/13 09:12:28, nicoe wrote:
> for key in keywords

Done.

http://codereview.tryton.org/62001/diff/1/qm.py#newcode150
qm.py:150: for item in self.from_items:
On 2011/07/13 09:12:28, nicoe wrote:
> You can do "', '.join(self.from_items)"

Done.

http://codereview.tryton.org/62001/diff/1/qm.py#newcode176
qm.py:176: return self.expected.__str__() + " " + self.operator.__str__() + " "
+ self.actual.__str__()
On 2011/07/13 09:12:28, nicoe wrote:
> 80 cols.
> Please use string interpolation

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld d9ca037-tainted