Merge lp://staging/~pr0gg3d/loggerhead/annotate_comment_notpresent_812583 into lp://staging/loggerhead

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

On 8/9/2011 9:38 AM, Francesco Del Degan wrote:
> Francesco Del Degan has proposed merging
> lp:~pr0gg3d/loggerhead/annotate_comment_notpresent_812583 into
> lp:loggerhead.
>
> Requested reviews: Loggerhead Reviewers (loggerhead-reviewers)
> Related bugs: Bug #812583 in loggerhead: "IndexError in
> add_template_values in loggerhead annotate"
> https://bugs.launchpad.net/loggerhead/+bug/812583
>
> For more details, see:
> https://code.launchpad.net/~pr0gg3d/loggerhead/annotate_comment_notpresent_812583/+merge/70825
>
> Fixes bug #812583 that raises an exception when annotating some
> lines where commit message is not present.
>
> It catch that exception (IndexError) and put the message as empty
> string.

Would it be possible to add a test for this bug? It shouldn't be too
hard to generate a commit without a comment and make sure that annotate
doesn't explode.

 review: needsfixing

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk5BAssACgkQJdeBCYSNAAMHMQCfYppjU8zVAVDBZrKwHoLkLZm5
m90AniY/yQB/+FmHSOWKfqWz93AJWSFT
=hjCf
-----END PGP SIGNATURE-----

Sure, i'll look into it.

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

On 8/9/2011 11:54 AM, Francesco Del Degan wrote:
> Sure, i'll look into it.

You should be able to add a test to loggerhead/tests/test_controllers.py

I think you can just set the 'rev_ids_texts' to whatever you want
(especially for your empty-file patch). You may want to add an optional
parameter to "make_annotate_ui_*" that takes a commit message, which you
can then set to be empty.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk5BLaUACgkQJdeBCYSNAAOdbQCgvVbqeRG8vy8ZXp3rH0pjR/cw
csAAnRZphxP0XfPDnAxbFElu96dzJdX/
=Zoo5
-----END PGP SIGNATURE-----

ya I refactored adding a third argument to 'history' tuple and modified the existing test adding two "." as messages and use one '' in void-comment test.

Now, i've to do the same mod into the other mp, extending "make_annotate_ui_*" and modifying the test for empty file.

I'm sorry, but i'm pretty new to bzr, i'm trying to figure out how handle this (sort of) conflict.

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

On 8/9/2011 3:08 PM, Francesco Del Degan wrote:
> ya I refactored adding a third argument to 'history' tuple and
> modified the existing test adding two "." as messages and use one ''
> in void-comment test.
>
> Now, i've to do the same mod into the other mp, extending
> "make_annotate_ui_*" and modifying the test for empty file.
>
> I'm sorry, but i'm pretty new to bzr, i'm trying to figure out how
> handle this (sort of) conflict.

I'm not sure how you are defining conflict. However, I would probably
just merge this branch into the other branch, and add the new updates.

Then when proposing it, you can mark that the other branch has a
"prerequisite" branch of this one. So the diff will show just what will
be newly introduced.

I would probably reduce the amount of the test that you copied. You
don't need the "set up by __call__" comment, etc. Just call get_values,
and add a comment that you are testing it handles an empty commit
message without breaking.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk5BMvkACgkQJdeBCYSNAAPpRgCdEU727Id3BujMxhuLp9nJbU0c
dSgAn3GmmNaOGADYgCZfDYLl0JjxJcHC
=hcFh
-----END PGP SIGNATURE-----

> I'm not sure how you are defining conflict. However, I would probably
> just merge this branch into the other branch, and add the new updates.
>
> Then when proposing it, you can mark that the other branch has a
> "prerequisite" branch of this one. So the diff will show just what will
> be newly introduced.
>
> I would probably reduce the amount of the test that you copied. You
> don't need the "set up by __call__" comment, etc. Just call get_values,
> and add a comment that you are testing it handles an empty commit
> message without breaking.

I made some cleanup on the code and I will merge this into the other branch.
When i will propose the other branch, i will mark this as a prerequisite.