> > # All clear on the custom module checks. Import some standard stuff.
> > -import getopt, os, popen2, pickle, datetime
> > +import getopt, os, popen2, pickle, datetime, re, string
>
> Scripts written for modern versions of Python almost never need 'string'
> imported explicitly. Details below.
>
Ok. Now, i don't use 're' and 'string' modules. So, this line can be
removed.
> rss = pickle.load(open(self.pickle_file, "r"))
> rss.items.insert(0, self.rss_item)
> if len(rss.items) > self.max_items:
> del(rss.items[self.max_items:])
>
Yes, i've made the correction.
> There are a number of problems with new code. First, it uses a two-space
> indentation, whereas the local policy is four-space indentation. Secondly,
> there are instances of function-space-paren, where local policy dictate no
> spaces between function name and parenthesis.
Addressed.
> You're using a regexp compilation and search for a single character (":"),
> when you could just do:
>
> elif commit_rev.find(":") == -1:
Addressed. Now, i'm not using a regexp compilation.
> else:
> start, end = commit_rev.split(':')
Addressed. Now, i'm not using the 'string' module.
> In fact, because it's okay to split on a character that isn't in the string,
> you can simplify the whole of the revision input handling like so:
>
I guess, it is not okay to split on a character that isn't in the
string. If it is 'None', it reports:
AttributeError: 'NoneType' object has no attribute 'split'
If it is not defined, it reports:
NameError: name 'revision' is not defined
So, i'm doing this:
if (commit_rev == None):
# TODO: do the 'svnlook youngest' thing
else:
rev_range = commit_rev.split(':')
len_rev_range = len(rev_range)
if (len_rev_range == 1):
revisions = [int(commit_rev)]
elif (len_rev_range == 2):
start, end = rev_range
start = int(start)
end = int(end)
> else:
> usage_and_exit("Revision specification '%s' is invalid." \
> % (commit_rev))
To maintain the uniformity across the script, i changed it as:
else:
usage_and_exit("svn2rss.py: Invalid value '%s' for \
--revision." % (commit_rev))
Please find attached the revised patch. Thank you!
[[[
* contrib/hook-scripts/svn2rss.py
(__init__): Add 'try' block and handle ValueError for revision. Here,
if the revision is not set, set it to the youngest one. Handle one
and range of revisions. Finally, for each revision, add the rss feed
to the file.
(SVN2RSS.__init__): Pop the rss items from the feed, while length of
rss feed >= max items
]]]
--
Regards,
Bhuvaneswaran
Received on Tue Jun 20 12:33:33 2006