Tool for printing changelog descriptions.

Tool for printing changelog descriptions.

This is similar to gencl but supports more features and is written in Python.

BUG=chromium-os:8205
TEST=Ran ( ./cros_changelog 0.9.102.0 cros/master > /tmp/test1.html && 
           ./cros_changelog 0.9.102.0 > /tmp/test2.html )
     First command prints changes between 0.9.102.0 and master. Second
     command prints changes between 0.9.102.0 and previous release.

[David McMahon, 2010-10-28 02:30:03]

On 2010/10/28 02:23:38, davidjames wrote:
> 

Nice!  Does this have any dependency on an existing checkout?  It appears to use
http for all it's queries?

Can you provide a text mode similar to gencl with an optional html mode?

[diandersAtChromium, 2010-10-28 18:03:33]

I can't test this patch myself.  Looks like there is a dependency on some
changes to "cros_build_lib.py"?  I assume that is submitted as a separate
review?

[diandersAtChromium, 2010-10-28 20:36:15]

http://codereview.chromium.org/4175007/diff/1/2
File chromite/bin/cros_changelog (right):

http://codereview.chromium.org/4175007/diff/1/2#newcode28
chromite/bin/cros_changelog:28: """Returns list of tags from current git
repository."""
It would be a lot easier to understand / make changes if python was used for
data munging instead of awk/sed/sort.  This should be some pretty simple python
code.

http://codereview.chromium.org/4175007/diff/1/2#newcode44
chromite/bin/cros_changelog:44: """Create commit logs."""
Seems like parameters should be documented here?

http://codereview.chromium.org/4175007/diff/1/2#newcode55
chromite/bin/cros_changelog:55: """Get bug ID from commit logs."""
Describe inputs and outputs for this function: inputs are object members and
output is return value (list of strings)

http://codereview.chromium.org/4175007/diff/1/2#newcode56
chromite/bin/cros_changelog:56: 
Some comments about what this is doing would be appreciated.  I'm not sure
exactly what the 'BUG=' syntax looks like and trying to figure it out properly
from the code below is tricky.

http://codereview.chromium.org/4175007/diff/1/2#newcode106
chromite/bin/cros_changelog:106: url_fmt =
'http://chromiumos-git/git/?p=%s.git;a=commitdiff;h=%s'
I'm not sure I understand this comment.  I think that the URL is supposed to be
a web URL and the gitrw.chromium.org:9222 is a ssh URL, no?

http://codereview.chromium.org/4175007/diff/1/2#newcode134
chromite/bin/cros_changelog:134: """Return list of commits to path between tag1
and tag2."""
Would be nice to see parameters / return values commented explicitly.

http://codereview.chromium.org/4175007/diff/1/2#newcode169
chromite/bin/cros_changelog:169: if not tag1:
Should be "if tag1 is None" unless you really wanted to catch the empty string
too.

http://codereview.chromium.org/4175007/diff/1/2#newcode170
chromite/bin/cros_changelog:170: tag1 = tags[tags.index(tag2) + 1]
Potential out of bounds issue here if tag2 is the last element in the list.

[David James, 2010-10-28 20:52:52]

These are good suggestions. Since we're planning to hand over the script to you,
implementing your suggestions would be a good way to get in your first change to
the script.

http://codereview.chromium.org/4175007/diff/1/2
File chromite/bin/cros_changelog (right):

http://codereview.chromium.org/4175007/diff/1/2#newcode28
chromite/bin/cros_changelog:28: """Returns list of tags from current git
repository."""
On 2010/10/28 20:36:15, diandersAtChromium wrote:
> It would be a lot easier to understand / make changes if python was used for
> data munging instead of awk/sed/sort.  This should be some pretty simple
python
> code.

Good idea. Could you add a TODO for this? This code is copied from gencl and
works well, but I think it should be rewritten in Python if we ever need to
change it or make it more complex.

http://codereview.chromium.org/4175007/diff/1/2#newcode44
chromite/bin/cros_changelog:44: """Create commit logs."""
On 2010/10/28 20:36:15, diandersAtChromium wrote:
> Seems like parameters should be documented here?

+1

http://codereview.chromium.org/4175007/diff/1/2#newcode55
chromite/bin/cros_changelog:55: """Get bug ID from commit logs."""
On 2010/10/28 20:36:15, diandersAtChromium wrote:
> Describe inputs and outputs for this function: inputs are object members and
> output is return value (list of strings)

+1

http://codereview.chromium.org/4175007/diff/1/2#newcode56
chromite/bin/cros_changelog:56: 
On 2010/10/28 20:36:15, diandersAtChromium wrote:
> Some comments about what this is doing would be appreciated.  I'm not sure
> exactly what the 'BUG=' syntax looks like and trying to figure it out properly
> from the code below is tricky.

+1. We should also mention it's copied from bugdroid:
http://src.chromium.org/viewvc/chrome/trunk/tools/bugdroid/bugdroid.py?revisi...

http://codereview.chromium.org/4175007/diff/1/2#newcode106
chromite/bin/cros_changelog:106: url_fmt =
'http://chromiumos-git/git/?p=%s.git;a=commitdiff;h=%s'
On 2010/10/28 20:36:15, diandersAtChromium wrote:
> I'm not sure I understand this comment.  I think that the URL is supposed to
be
> a web URL and the gitrw.chromium.org:9222 is a ssh URL, no?

Yeah. That was a misunderstanding. djmm and I discussed this in person.

http://codereview.chromium.org/4175007/diff/1/2#newcode134
chromite/bin/cros_changelog:134: """Return list of commits to path between tag1
and tag2."""
On 2010/10/28 20:36:15, diandersAtChromium wrote:
> Would be nice to see parameters / return values commented explicitly.

+1

http://codereview.chromium.org/4175007/diff/1/2#newcode169
chromite/bin/cros_changelog:169: if not tag1:
On 2010/10/28 20:36:15, diandersAtChromium wrote:
> Should be "if tag1 is None" unless you really wanted to catch the empty string
> too.

Empty string is bad too, so let's leave this as-is.

http://codereview.chromium.org/4175007/diff/1/2#newcode170
chromite/bin/cros_changelog:170: tag1 = tags[tags.index(tag2) + 1]
On 2010/10/28 20:36:15, diandersAtChromium wrote:
> Potential out of bounds issue here if tag2 is the last element in the list.

+1, please fix