[MERGE] tags in log output

[Kent Gibson]

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

Hi Alexander,

I've got a comment that isn't specific to your patch, it's more about
plugin APIs in general, but your patch is a specific example.

Rather than changing the LogFormatter API every time some new detail
is added to a revision (tags in this case, the delta for merge revs in
a recent patch from JAM), wouldn't it be better to make the tags an
attribute of the revision itself?

As I see it the LF API should simply be:

start_log()
log_revision(revision)
end_log()

The start_log and end_log are there for formatters, such as XML, that
need bookends.
The log_revision is called for each revision to be logged.

So long as we guarantee to only add attributes to the revision, not
change or delete them, we are guaranteed to be backwardly compatible
with plugin log formatters.

That would get rid of this sort of ugliness where an ever growing
laundry list of attributes is being passed:
+                    lf.show_merge_revno(rev, merge_depth, revno,
+                                        rev_tag_dict.get(rev_id))

and also the hideous code that tests the capabilities of the LF to
work out how to call it:
+    use_tags = getattr(lf, 'supports_tags', False)
...
+                if use_tags:
+                    lf.show_merge_revno(rev, merge_depth, revno,
+                                        rev_tag_dict.get(rev_id))
+                else:
+                    lf.show_merge_revno(rev, merge_depth, revno)


I can see the usefulness of testing the LF capabilities to determine
what revision attributes to populate, but I would seriously avoid
using it to guess the API itself.

Cheers,
Kent.


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

iD8DBQFGHju1goxTFTi1P8QRAvt8AJ4rjseV+b2Ro8QgSi2BHJUKhieTaQCgq0Xt
1r51THmmqro14qK6IJe+j4w=
=EG56
-----END PGP SIGNATURE-----