Daniel Shahaf wrote:
> Edmund Wong wrote on Mon, 14 Sep 2009 at 01:36 +0800:
>> [[[
>>
>> Fixed gen_win.py such that it checks for the presence of the
>> Serf library (which it does prior to this patch) and also
>> outputs the library version (which is what this patch does).
>>
>> * build/generator/gen_win.py
>> (_get_serf_version): New function to return the serf library
>> version in a 3-tuple format.
>
> The '(' should be indented two spaces.
I'll get this fixed.
>
>> (_find_serf): Added a version check. If the 3-tuple version
>> structure has -1 in the first index, the serf library is
>> not built; otherwise, the library is set to be built. Then
>> a message is displayed informing the user of the results.
>>
>
> IMO, the part about -1 and 3-tuple is a low-enough implementation detail
> to not belong in the log message. Just say what the added code thus
> (extract the version, message if successful, skip building(?) otherwise).
I see. I'll modify that part of the log message.
>> +
>> + if self.serf_path and os.path.exists(self.serf_path):
>> + if (os.path.exists(os.path.join(self.serf_path, 'serf.h'))):
>> + fp = open(os.path.join(self.serf_path, 'serf.h'))
>> + txt = fp.read()
>> + maj_t = re.compile(r'SERF_MAJOR_VERSION (\d+)')
>> + min_t = re.compile(r'SERF_MINOR_VERSION (\d+)')
>> + patch_t = re.compile(r'SERF_PATCH_VERSION (\d+)')
>> + ver_maj_re = maj_t.search(txt)
>> + ver_min_re = min_t.search(txt)
>> + ver_patch_re = patch_t.search(txt)
>
> This reads the entire file to the variable 'txt'. It would be better to
> read it incrementally (e.g., one line at a time).
I just realized i didn't even close the file.
>
> (And perhaps relax the regexes by allowing any number of whitespace
> there? This is almost a matter of personal taste, though.)
That is a point. Regexes isn't my strong point. So would the
following suffice?
maj_t = re.compile(r'SERF_MAJOR_VERSION(\s+)(\d+)')
Edmund
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2394394
Received on 2009-09-14 02:56:13 CEST