On 09/25/2014 03:30 AM, Fraser Tweedale wrote:
On Wed, Sep 24, 2014 at 09:16:52AM -0500, Endi Sukma Dewata wrote:
On 9/24/2014 8:26 AM, Petr Vobornik wrote:
On 24.9.2014 04:43, Endi Sukma Dewata wrote:
On 9/22/2014 9:49 AM, Petr Vobornik wrote:
[PATCH] webui-ci: case-insensitive record check
Indirect association are no longer lower cased, which caused a issue
in CI.
Is the use of "|=" operator intentional? I don't see the "has" variable
defined anywhere else in this method.
has |= self.has_record(pkey.lower(), parent, table_name)
If this has been tested to work, then ACK.
it's defined on the previous line:
has = self.has_record(pkey, parent, table_name)
has |= self.has_record(pkey.lower(), parent, table_name)
Ah, I see. I thought the old line was being replaced.
ACK.
IMO the following reads better::
has = self.has_record(pkey, parent, table_name) \
|| self.has_record(pkey.lower(), parent, table_name)
(Just a comment - no reason to block the patch.)
Feel free to push the patch as it is, but since we're debating style...
Write this:
has_record = (self.has_record(pkey, parent, table_name) or
self.has_record(pkey.lower(), parent, table_name))
For boolean values, you should prefer `or` and `and` over the bitwise
operators, since "truthy"/"falsy" values might not be just booleans.
It's better illustrated with `and`: `3 and 8` is true, but `3 & 8` is false.
And from PEP8:
- The preferred way of wrapping long lines is by using Python's implied
line continuation inside parentheses, brackets and braces. Long lines
can be broken over multiple lines by wrapping expressions in
parentheses. These should be used in preference to using a backslash for
line continuation.
- The preferred place to break around a binary operator is after the
operator, not before it.
--
Petr³
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
