[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] svn2rss.py: --revision takes a range (m:n)

From: Bhuvaneswaran Arumugam <bhuvan_at_collab.net>
Date: 2006-06-20 12:32:46 CEST

> > # 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

This is an archived mail posted to the Subversion Dev mailing list.