You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Senthil Kumaran S <se...@collab.net> on 2008/07/15 09:13:48 UTC

[PATCH] Do not print log separator in contribulyze.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

As per the following IRC conversation, I am attaching this patch to fix
contribulyze.py script, which will not print log separator line if there is no
"\n" (at the end) in the log message:

<snip>
(11:26:38  IST) danielsh: stylesen: morning :) what was the purpose of the
propedits you did this afternoon?
(11:28:01  IST) Arfrever: danielsh: I guess that he added missing lines at the
end of log messages.
(11:28:26  IST) stylesen: danielsh: Good night :) they did not have a final
new line, which appeared a bit ugly when I did 'svn log | less' on the 1.5.x
branch, but later after 2 or 3 revisions I realized, there are lot of log
messages like that, so stopped after 2 or 3 revisions!
(11:28:54  IST) danielsh: stylesen: ah, thanks.
(11:29:15  IST) ***danielsh has autocmd FileType svn set noendofline...
(English: all my log messages don't have terminating newline)
(11:30:47  IST) stylesen: danielsh: A basic problem when we dont have final
new line is in contriubulize where after the log message the '--------------'
line also appears in the listing
(11:31:20  IST) danielsh: stylesen: it's a bug in contribulyze.py, then.
(11:31:42  IST) stylesen: danielsh: okie will fix it :)
(11:32:16  IST) danielsh: stylesen: cool :)
</snip>

[[[
Do not print log separator in contribulyze.

* tools/dev/contribulyze.py
  (graze): If there is no "\n" at the end of a log message, do not print
   the log separator.

Patch by: stylesen
]]]

Thank You.

PS: Some lines which are not relevant to this patch are stripped in the above
IRC conversation.

- --
Senthil Kumaran S
http://www.stylesen.org/

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFIfGpJ9o1G+2zNQDgRArvPAJ4hk5oXooUPpSs9lYHIqE2zi4d0OQCeOieC
hTLyo+9wUvai7oZidpCBu5Q=
=fHiz
-----END PGP SIGNATURE-----

Re: [PATCH] Do not print log separator in contribulyze.

Posted by Senthil Kumaran S <se...@collab.net>.
Senthil Kumaran S wrote:
> Updated the log message, but I am not sure whether this is the right way 
> to include IRC transcript. I used kf-irc-prettify 
> (http://svn.red-bean.com/repos/kfogel/trunk/.emacs) and some manual 
> edition in order to format the IRC log message.

Forgot to attach patch in previous email. Doing it now.

Thank You.
-- 
Senthil Kumaran S
http://www.stylesen.org/


Re: [PATCH] Do not print log separator in contribulyze.

Posted by Senthil Kumaran S <se...@collab.net>.
Karl Fogel wrote:
> Committed in r32147, thanks.

Thank You Karl.

> Just for next time: it's best to always post log message and patch
> together, so no one has to think about which log message goes with which
> patch.  Even when there is only one patch and multiple log messages, as
> in this case, it's still easy to get confused.  The goal to aim for is
> one single mail message that overrides all previous mails, and says so
> clearly.  That way, the most thinking anyone has to do is to compare
> dates on messages.

Will do it in future :)

> By the way, I think Peter Samuelson was right, in this case: the IRC
> transcript wasn't adding much information after all, so I used a
> different log message.

I created some noise before reading my entire mailbox ;)

-- 
Senthil Kumaran S
http://www.stylesen.org/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Do not print log separator in contribulyze.

Posted by Karl Fogel <kf...@red-bean.com>.
Committed in r32147, thanks.

Just for next time: it's best to always post log message and patch
together, so no one has to think about which log message goes with which
patch.  Even when there is only one patch and multiple log messages, as
in this case, it's still easy to get confused.  The goal to aim for is
one single mail message that overrides all previous mails, and says so
clearly.  That way, the most thinking anyone has to do is to compare
dates on messages.

By the way, I think Peter Samuelson was right, in this case: the IRC
transcript wasn't adding much information after all, so I used a
different log message.

-Karl

Senthil Kumaran S <se...@collab.net> writes:
> Senthil Kumaran S wrote:
>> Hi Karl,
>>
>> Karl Fogel wrote:
>>> It's okay to include that IRC transcript in your log message, to give
>>> the reader an understanding of what motivates the change.
>>
>> Updated the log message, but I am not sure whether this is the right
>> way to include IRC transcript. I used kf-irc-prettify
>> (http://svn.red-bean.com/repos/kfogel/trunk/.emacs) and some manual
>> edition in order to format the IRC log message.
>
> The log message which I gave in my previous email contains some
> characters in different encoding inside the IRC transcript, corrected
> that and here is the new log message (sorry for the noise).
>
> [[[
> Do not print log separator in contribulyze.
>
> * tools/dev/contribulyze.py
>   (graze): If there is no final newline at the end of a log message,
>    move to the next log message.
>
> IRC transcript:
>     danielsh: stylesen: morning :) what was the purpose of the propedits
>      you did this afternoon?
>
>     Arfrever: danielsh: I guess that he added missing lines at the end
>      of log messages.
>
>     stylesen: danielsh: Good night :) they did not have a final n
>      line, which appeared a bit ugly when I did 'svn log | less' on the
>      1.5.x branch, but later after 2 or 3 revisions I realized, there
>      are lot of log messages like that, so stopped after 2 or 3 revisions!
>
>     danielsh: stylesen: ah, thanks.
>
>     ***danielsh has autocmd FileType svn set noendofline...
>      (English: all my log messages don't have terminating newline)
>
>     stylesen: danielsh: A basic problem when we dont have final n
>      line is in contriubulize where after the log message the
>      '--------------' line also appears in the listing
>
>     danielsh: stylesen: it's a bug in contribulyze.py, then.
>
>     stylesen: danielsh: okie will fix it :)
>
>     danielsh: stylesen: cool :)
>
> Patch by: stylesen
> ]]]
>
> Thank You.
> --
> Senthil Kumaran S
> http://www.stylesen.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Do not print log separator in contribulyze.

Posted by Senthil Kumaran S <se...@collab.net>.
Senthil Kumaran S wrote:
> Hi Karl,
> 
> Karl Fogel wrote:
>> It's okay to include that IRC transcript in your log message, to give
>> the reader an understanding of what motivates the change.
> 
> Updated the log message, but I am not sure whether this is the right way 
> to include IRC transcript. I used kf-irc-prettify 
> (http://svn.red-bean.com/repos/kfogel/trunk/.emacs) and some manual 
> edition in order to format the IRC log message.

The log message which I gave in my previous email contains some characters in 
different encoding inside the IRC transcript, corrected that and here is the 
new log message (sorry for the noise).

[[[
Do not print log separator in contribulyze.

* tools/dev/contribulyze.py
   (graze): If there is no final newline at the end of a log message,
    move to the next log message.

IRC transcript:
     danielsh: stylesen: morning :) what was the purpose of the propedits
      you did this afternoon?

     Arfrever: danielsh: I guess that he added missing lines at the end
      of log messages.

     stylesen: danielsh: Good night :) they did not have a final n
      line, which appeared a bit ugly when I did 'svn log | less' on the
      1.5.x branch, but later after 2 or 3 revisions I realized, there
      are lot of log messages like that, so stopped after 2 or 3 revisions!

     danielsh: stylesen: ah, thanks.

     ***danielsh has autocmd FileType svn set noendofline...
      (English: all my log messages don't have terminating newline)

     stylesen: danielsh: A basic problem when we dont have final n
      line is in contriubulize where after the log message the
      '--------------' line also appears in the listing

     danielsh: stylesen: it's a bug in contribulyze.py, then.

     stylesen: danielsh: okie will fix it :)

     danielsh: stylesen: cool :)

Patch by: stylesen
]]]

Thank You.
-- 
Senthil Kumaran S
http://www.stylesen.org/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Do not print log separator in contribulyze.

Posted by Senthil Kumaran S <se...@collab.net>.
Hi Karl,

Karl Fogel wrote:
> It's okay to include that IRC transcript in your log message, to give
> the reader an understanding of what motivates the change.

Updated the log message, but I am not sure whether this is the right way to 
include IRC transcript. I used kf-irc-prettify 
(http://svn.red-bean.com/repos/kfogel/trunk/.emacs) and some manual edition in 
order to format the IRC log message.

[[[
Do not print log separator in contribulyze.

* tools/dev/contribulyze.py
   (graze): If there is no final newline at the end of a log message,
    move to the next log message.

IRC transcript:
     danielsh: stylesen: morning :) what was the purpose of the propedits
      you did this afternoon?

     Arfrever: danielsh: I guess that he added missing lines at the end
      of log messages.

     stylesen: danielsh: Good night :) they did not have a final n
      line, which appeared a bit ugly when I did 'svn log | less' on the
      1.5.x branch, but later after 2 or 3 revisions I realized, there
      are lot of log messages like that, so stopped after 2 or 3 revisions!

     danielsh: stylesen: ah, thanks.

     ***danielsh has autocmd FileType svn set noendofline...
      (English: all my log messages don't have terminating newline)

     stylesen: danielsh: A basic problem when we dont have final n
      line is in contriubulize where after the log message the
      '--------------' line also appears in the listing

     danielsh: stylesen: it's a bug in contribulyze.py, then.

     stylesen: danielsh: okie will fix it :)

     danielsh: stylesen: cool :)

Patch by: stylesen
]]]

> I'm confused, though: your log messages says "If there is no '\n' ... do
> not print the log separator".  But then your code change does something
> else: it *replaces* the encountered log_separator with a newline.  How
> does it avoid combining two log messages into one?  The patch never sets
> 'just_saw_separator' or does any of the other "we just encountered a
> boundary" stuff... However, when I tested, I didn't see any log entries
> get combined, so it might be that your patch is correct and I just don't
> understand why :-).

That was a quick fix :) and I found a new bug in that, thanks for pointing it
out ie., if there is a log message immediately after the log message which has
no final newline, then we don't get the log message following it (the log
message without final newline).

For example:
------------------------------------------------------------------------
r31726 | cmpilato | 2008-06-13 18:19:34 +0530 (Fri, 13 Jun 2008) | 8 lines
Changed paths:
    M /trunk/Makefile.in

Make install-javahl depend on javahl.

* Makefile.in
   (install-javahl): Add javahl as a dependency for installing javahl. This
     ensures building javahl before we install.

Patch by: stylesen
------------------------------------------------------------------------
r21246 | cmpilato | 2006-08-24 21:06:57 +0530 (Thu, 24 Aug 2006) | 13 lines
Changed paths:
    M /trunk/contrib/hook-scripts/svn2feed.py

Place item author and date information in feed metadata.

* contrib/hook-scripts/svn2feed.py
   (global): Remove the TODO item.
   (Svn2Feed._get_item_dict): Remove the date information from desc to
     avoid redundancy.  Strip the newline and store the author information
     in data structure.
   (Svn2Feed._format_update_ts): New helper function.
   (Svn2RSS._make_rss_item): Pass author information to RSSItem function.
   (Svn2Atom._make_atom_item): Create a new tag for author and append it
     to the entry.

Patch by: Bhuvaneswaran Arumugam <bh...@collab.net>
------------------------------------------------------------------------

In the above log message r21246 will not appear in the output as per my first
patch.

> But in any case, when I tried to reproduce the bug without your patch, I
> couldn't.  Could you give a formal reproduction recipe?

This is because in your log-data.in for every log message which did not have a
final newline you had something like "(some text)" at the end which is handled
by the following code:

<snip>
                 line = input.readline()
                 log.accum(line)
                 num_lines -= 1
                 m = in_field_re.match(line)
                 if not m:
                   m = field_re.match(line)
                   if not m:
                     aside_match = parenthetical_aside_re.match(line)
                     if aside_match:
                       field.add_endum(line)
                   log.add_field(field)
                   field = None
</snip>

Note the "parenthetical_aside_re.match(line)" above which prevented you from
reproducing this issue.

Now with my updated patch attached we do not get log separator printed and
other functionalities are working fine.

Steps to reproduce (run with and without the updated patch and observe
stylesen's logs):

	$ cd tools/dev
	$ ./contribulyze.py -C ../../COMMITTERS < log-data.in

log-data contents are as follows:

------------------------------------------------------------------------
r32124 | pburba | 2008-07-15 10:04:04 -0400 (Tue, 15 Jul 2008) | 2 lines

This is a fake log message for r32124, to test out contribulyze.py.

------------------------------------------------------------------------
r32123 | sunny256 | 2008-07-15 06:13:49 -0400 (Tue, 15 Jul 2008) | 4 lines

This is a fake log message for r32123, to test out contribulyze.py.

Patch by: J. Random Contributor <jr...@example.com>
(Tweaked by me.  Look, this log msg doesn't have its own final newline!)
------------------------------------------------------------------------
r32122 | pburba | 2008-07-14 21:25:56 -0400 (Mon, 14 Jul 2008) | 5 lines

This is a fake log message for r32122, to test out contribulyze.py.

Patch by: Some Other Contributor <an...@example.com>
(Tweaked by me.  This log msg does have its own final newline.)

------------------------------------------------------------------------
r32121 | pburba | 2008-07-14 20:47:11 -0400 (Mon, 14 Jul 2008) | 3 lines

This is a fake log message for r32121, to test out contribulyze.py.

Patch by: stylesen
------------------------------------------------------------------------
r32120 | pburba | 2008-07-14 20:39:58 -0400 (Mon, 14 Jul 2008) | 2 lines

This is a fake log message for r32120, to test out contribulyze.py.

------------------------------------------------------------------------
r32119 | pburba | 2008-07-14 19:37:15 -0400 (Mon, 14 Jul 2008) | 2 lines

This is a fake log message for r32119, to test out contribulyze.py.

------------------------------------------------------------------------

Thank You.
-- 
Senthil Kumaran S
http://www.stylesen.org/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Do not print log separator in contribulyze.

Posted by Senthil Kumaran S <se...@collab.net>.
Senthil Kumaran S wrote:
>> Oh, I don't know ... what you really want to say is "Fix contribulyze
>> output to account for the fact that log entries may or may not end with
>> \n."  It seems to me that the IRC transcript would add a great many
>> words but no useful information.
> 
> Does the following log message looks good (I ve removed the IRC 
> transcript since it looks ugly and also does not provide needed info as 
> you pointed out):
> 
> [[[
> Fix contribulyze output to account for the fact that log entries may
> or may not end with '\n'.
> 
> * tools/dev/contribulyze.py
>   (graze): If there is no final newline at the end of a log message,
>    move to the next log message.
> 
> Patch by: stylesen
> ]]]

Sorry for the noise, I didnt notice that this patch was committed by kfogel in 
r32147 and svn:log edited by blair later.

Just now saw my commit emails!

-- 
Senthil Kumaran S
http://www.stylesen.org/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Do not print log separator in contribulyze.

Posted by Senthil Kumaran S <se...@collab.net>.
Peter Samuelson wrote:
> [Karl Fogel]
>> It's okay to include that IRC transcript in your log message, to give
>> the reader an understanding of what motivates the change.
> 
> Oh, I don't know ... what you really want to say is "Fix contribulyze
> output to account for the fact that log entries may or may not end with
> \n."  It seems to me that the IRC transcript would add a great many
> words but no useful information.

Does the following log message looks good (I ve removed the IRC transcript 
since it looks ugly and also does not provide needed info as you pointed out):

[[[
Fix contribulyze output to account for the fact that log entries may
or may not end with '\n'.

* tools/dev/contribulyze.py
   (graze): If there is no final newline at the end of a log message,
    move to the next log message.

Patch by: stylesen
]]]

Thank You.
-- 
Senthil Kumaran S
http://www.stylesen.org/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Do not print log separator in contribulyze.

Posted by Peter Samuelson <pe...@p12n.org>.
[Karl Fogel]
> It's okay to include that IRC transcript in your log message, to give
> the reader an understanding of what motivates the change.

Oh, I don't know ... what you really want to say is "Fix contribulyze
output to account for the fact that log entries may or may not end with
\n."  It seems to me that the IRC transcript would add a great many
words but no useful information.
-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Do not print log separator in contribulyze.

Posted by Karl Fogel <kf...@red-bean.com>.
Senthil Kumaran S <se...@collab.net> writes:
> [[[
> Do not print log separator in contribulyze.
>
> * tools/dev/contribulyze.py
>   (graze): If there is no "\n" at the end of a log message, do not print
>    the log separator.
>
> Patch by: stylesen
> ]]]

It's okay to include that IRC transcript in your log message, to give
the reader an understanding of what motivates the change.

I'm confused, though: your log messages says "If there is no '\n' ... do
not print the log separator".  But then your code change does something
else: it *replaces* the encountered log_separator with a newline.  How
does it avoid combining two log messages into one?  The patch never sets
'just_saw_separator' or does any of the other "we just encountered a
boundary" stuff... However, when I tested, I didn't see any log entries
get combined, so it might be that your patch is correct and I just don't
understand why :-).

But in any case, when I tried to reproduce the bug without your patch, I
couldn't.  Could you give a formal reproduction recipe?

The command line I used was:

   $ cd tools/dev
   $ ./contribulyze.py -U 'http://svn.collab.net/viewcvs/svn?rev=%s&view=rev' -C ../../COMMITTERS < log-data.in

And log-data.in contains this:

------------------------------------------------------------------------
r32124 | pburba | 2008-07-15 10:04:04 -0400 (Tue, 15 Jul 2008) | 2 lines

This is a fake log message for r32124, to test out contribulyze.py.

------------------------------------------------------------------------
r32123 | sunny256 | 2008-07-15 06:13:49 -0400 (Tue, 15 Jul 2008) | 4 lines

This is a fake log message for r32123, to test out contribulyze.py.

Patch by: J. Random Contributor <jr...@example.com>
(Tweaked by me.  Look, this log msg doesn't have its own final newline!)
------------------------------------------------------------------------
r32122 | pburba | 2008-07-14 21:25:56 -0400 (Mon, 14 Jul 2008) | 5 lines

This is a fake log message for r32122, to test out contribulyze.py.

Patch by: Some Other Contributor <an...@example.com>
(Tweaked by me.  This log msg does have its own final newline.)

------------------------------------------------------------------------
r32121 | pburba | 2008-07-14 20:47:11 -0400 (Mon, 14 Jul 2008) | 2 lines

This is a fake log message for r32121, to test out contribulyze.py.

------------------------------------------------------------------------
r32120 | pburba | 2008-07-14 20:39:58 -0400 (Mon, 14 Jul 2008) | 2 lines

This is a fake log message for r32120, to test out contribulyze.py.

------------------------------------------------------------------------
r32119 | pburba | 2008-07-14 19:37:15 -0400 (Mon, 14 Jul 2008) | 2 lines

This is a fake log message for r32119, to test out contribulyze.py.

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org