You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bhuvaneswaran Arumugam <bh...@collab.net> on 2006/06/18 05:56:45 UTC
[PATCH] svn2rss.py: --revision takes a range (m:n)
Hello,
I have attached a patch for svn2rss.py python script. The patch contains
following fixes and they are related to each other:
1) --revision takes a range (m:n), like the 'svn' client does
2) if the revision is not set, it is set to the youngest one
[[[
--revision takes a range (m:n), like the 'svn' client does and default
value is set to the youngest one
* svn2rss.py
(import): import re, string
(__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
Re: [PATCH] svn2rss.py: --revision takes a range (m:n)
Posted by Bhuvaneswaran Arumugam <bh...@collab.net>.
> [[[
> * 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
> ]]]
Mike have tweaked and committed this patch in r20310.
--
Regards,
Bhuvaneswaran
Re: [PATCH] svn2rss.py: --revision takes a range (m:n)
Posted by Bhuvaneswaran Arumugam <bh...@collab.net>.
> > # 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
Re: [PATCH] svn2rss.py: --revision takes a range (m:n)
Posted by "C. Michael Pilato" <cm...@collab.net>.
Bhuvaneswaran Arumugam wrote:
> Hello,
>
> I have attached a patch for svn2rss.py python script. The patch contains
> following fixes and they are related to each other:
>
> 1) --revision takes a range (m:n), like the 'svn' client does
> 2) if the revision is not set, it is set to the youngest one
>
> [[[
> --revision takes a range (m:n), like the 'svn' client does and default
> value is set to the youngest one
>
> * svn2rss.py
> (import): import re, string
I'd leave the "(import): " part off of this line. You aren't really
modifying a symbol named "import" ... this is the equivalent of adding a
#include in C code, so:
* contrib/hook-scripts/svn2rss.py
Import 're' and 'string' modules.
(__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
> Index: contrib/hook-scripts/svn2rss.py
> ===================================================================
> --- contrib/hook-scripts/svn2rss.py (revision 20157)
> +++ contrib/hook-scripts/svn2rss.py (working copy)
> @@ -34,7 +34,7 @@
> sys.exit(1)
>
> # 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.
> @@ -152,8 +153,10 @@
> f = open(self.pickle_file, "r")
> rss = pickle.load(f)
> f.close()
> - if len(rss.items) == self.max_items :
> + length = len(rss.items)
> + while (length >= self.max_items):
> rss.items.pop()
> + length = length - 1
> rss.items.insert(0, self.rss_item)
I wonder if this couldn't be done a little more cleverly. Something like
(NOTE: untested code):
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:])
> +try:
> + if (commit_rev is None):
> + cmd = "svnlook youngest " + repos_path
> + out, x, y = popen2.popen3(cmd)
> + cmd_out = out.readlines()
> + revisions = [int(cmd_out[0])]
> + out.close()
> + x.close()
> + y.close()
> + elif (not re.compile(":").search(commit_rev)):
> + revisions = [int(commit_rev)]
> + # if revision contains ":", split and add the rss items for all
> + # revisions mentioned in the range
> + else:
> + start, end = string.split (commit_rev, ':')
> + start = int(start)
> + end = int(end)
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.
You're using a regexp compilation and search for a single character (":"),
when you could just do:
elif commit_rev.find(":") == -1:
Here's where your string module gets used, but a) you could just import that
module in the one logic fork that needs it:
else:
import string
start, end = string.split(commit_rev, ':')
or, you could avoid the import altogether:
else:
start, end = commit_rev.split(':')
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:
rev_range = commit_rev.split(':')
len_rev_range = len(rev_range)
if len_rev_range == 0:
# TODO: do the 'svnlook youngest' thang
elif len_rev_range == 1:
revisions = rev_range
elif len_rev_range == 2:
# The rss feed is a stack. Generate the rss item from 'end'
# revision, so it is moved down and 'start' revision remains as
# first rss item
if (start < end):
revisions = range(end, start - 1, -1)
else:
revisions = range(end , start + 1, 1)
else:
usage_and_exit("Revision specification '%s' is invalid." \
% (commit_rev))
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand