Issue 62001: First version of pysql
Patch Set 1 #
Total comments: 28
MessagesTotal messages: 6
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 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.
Sign in to reply to this message.
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 (but not inside function calls and definition)). See PEP8. http://codereview.tryton.org/62001/diff/1/qm.py#newcode21 qm.py:21: for expression in self.expressions: ", ".join(self.expressions) is better http://codereview.tryton.org/62001/diff/1/qm.py#newcode24 qm.py:24: expressions_to_print+=expression.__str__() expression.__str__() == str(expression) http://codereview.tryton.org/62001/diff/1/qm.py#newcode26 qm.py:26: if self.where!=[]: if self.where http://codereview.tryton.org/62001/diff/1/qm.py#newcode34 qm.py:34: self.str_params+="'"+param.__str__()+"'" What is this ? What are you quoting ? I don't understand. http://codereview.tryton.org/62001/diff/1/qm.py#newcode36 qm.py:36: if self.group_by!="": "if self.group_by" is better http://codereview.tryton.org/62001/diff/1/qm.py#newcode43 qm.py:43: if cpt_params!=0: "if cpt_params" is better http://codereview.tryton.org/62001/diff/1/qm.py#newcode51 qm.py:51: generated_string+=self.get_generated_string(self._cpt/26) get_generate_string does not take any argument http://codereview.tryton.org/62001/diff/1/qm.py#newcode89 qm.py:89: return_str=self.table_name + " AS " + self.alias The alias is not always filled http://codereview.tryton.org/62001/diff/1/qm.py#newcode91 qm.py:91: return_str+= join.__str__() Is it self.join ? http://codereview.tryton.org/62001/diff/1/qm.py#newcode96 qm.py:96: return value could return directly http://codereview.tryton.org/62001/diff/1/qm.py#newcode103 qm.py:103: self.join.table+other What does this do ? http://codereview.tryton.org/62001/diff/1/qm.py#newcode121 qm.py:121: for key in keys: for key in keywords http://codereview.tryton.org/62001/diff/1/qm.py#newcode150 qm.py:150: for item in self.from_items: You can do "', '.join(self.from_items)" http://codereview.tryton.org/62001/diff/1/qm.py#newcode176 qm.py:176: return self.expected.__str__() + " " + self.operator.__str__() + " " + self.actual.__str__() 80 cols. Please use string interpolation http://codereview.tryton.org/62001/diff/1/qm.py#newcode181 qm.py:181: def __or__(self,other): Add "not"
Sign in to reply to this message.
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.
|
