Re: [PATCH] svnlook.py: Make it usable as a library
From: anatoly techtonik <techtonik_at_gmail.com>
Date: Thu, 14 Nov 2013 02:58:21 +0300
Wow. Thanks for the review and committing the stuff so quick.
About dates, I need to check if the behaviour changed for myself.
> $ diff -bu3 svnlook.py svnlook.py\?revision\=1541558 -p
Because svnlook.py is used in hook scripts, it is usually copied
> @@ -76,7 +74,7 @@ class SVNLook(object):
I believe that this is critical for some API calls and arithmetic that
> def cmd_date(self):
I believe that svnlook.py output was different than of svnlook, but I may
> def get_date(self, unixtime=False):
What are other cases when date may be empty?
Anyway, LGTM without any more fixes.
-- anatoly t. On Wed, Nov 13, 2013 at 7:13 PM, Julian Foad <julianfoad_at_btopenworld.com> wrote: > (I sound so grumpy. Let me try again.) > > Thank you, Anatoly, for bothering to contribute your changes back to the community. > > Your main changes look good and well commented. I like them and I've committed them. > > It always makes it easier and thus more likely that someone will commit your patch if you supply a log message even for a simple change, but in this case I wrote one. (Afterwards, I saw that in GitHub you have a series of log messages for the changes that made up this patch, but even if I'd seen them earlier I would still have needed to combine them into a single Subversion-style message.) > > It's also best to avoid unrelated changes in the same patch. In this case, I stripped out the changes that seemed to be unrelated. Apologies if I missed something important. > > Thanks again. > - Julian > > > I (Julian Foad) wrote: > >> Hi, anatoly. >> >> Your patch contains: >> >> * make usable as a library by adding getter methods; >> >> * when printing a date, use a different formatting; >> >> * minor changes: >> adding a source code version label; >> setting self.rev to youngest rev even if a txn-id is provided; >> s/youngest/latest/ in the help. >> >> * no log message. >> >> The main changes (adding getter methods) look good. I have tested the changes by >> hand, written a log message, and committed: >> <http://svn.apache.org/r1541558>. >> >> The change to the display of dates is a separate issue. I don't really care >> what format it displays, but this code has a problem: it now displays a date >> even when there is no "svn:date" property. >> >> Are any of the minor changes important or useful? >> >> Thanks. > [...] >>>> https://github.com/apache/subversion/pull/1/files >>>> >>>> Please, CC.Received on 2013-11-14 00:59:13 CET
This is an archived mail posted to the Subversion Dev mailing list.