[MERGE] Clean up pull -v

Vincent Ladeuil v.ladeuil+lp at free.fr
Fri Dec 12 11:48:58 GMT 2008

>>>>> "aaron" == Aaron Bentley <aaron at aaronbentley.com> writes:

    aaron> Hi all,
    aaron> This patch cleans up the operations used in pull -v, so that they don't
    aaron> scale with the size of history, and so they're efficient with branch 6
    aaron> and later branch formats.  In particular, it uses
    aaron> repository.get_revisions, and does not use Branch.revision_history.

That's warmly welcome !

    aaron> Aaron
    aaron> # Bazaar merge directive format 2 (Bazaar 0.90)
    aaron> # revision_id: aaron at aaronbentley.com-20081212045246-mno3aaknnmgzmau9
    aaron> # target_branch: http://bazaar-vcs.org/bzr/bzr.dev
    aaron> # testament_sha1: e228f1d032dfa25e334e8b821c7b8f9c991fcc07
    aaron> # timestamp: 2008-12-11 20:53:59 -0800
    aaron> # source_branch: http://code.aaronbentley.com/bzr/bzrrepo/pull-log
    aaron> # base_revision_id: pqm at pqm.ubuntu.com-20081212025611-838ms79rnxjl7yr2
    aaron> # 
    aaron> # Begin patch
    aaron> === modified file 'NEWS'
    aaron> --- NEWS	2008-12-12 01:47:30 +0000
    aaron> +++ NEWS	2008-12-12 04:52:46 +0000
    aaron> @@ -97,6 +97,9 @@
    aaron>        asking for many texts that share ancestry, as the individual chunks
    aaron>        can be shared between versions of the file. (John Arbash Meinel)
    aaron> +    * ``pull -v`` uses new function ``bzrlib.log.show_branch_change``
    aaron> +      (Aaron Bentley)
    aaron> +

Introducing a new public function in bzrlib.log is not.

bzrlib.log is already quite messy and includes many functions
that we should deprecate in favor in a move unified API around
show_log (there is a TODO in show_changed_revisions which you
didn't copy when you create show_branch_change).

At a minimum, I'd like your new functions to be private, we
*plan* to deprecate show_changed_revisions(), it would be good to
*not* add new functions that will need to be deprecated at the
same time.
    aaron>  bzr 1.10 2008-12-05

    aaron> === modified file 'bzrlib/builtins.py'
    aaron> --- bzrlib/builtins.py	2008-12-11 06:03:57 +0000
    aaron> +++ bzrlib/builtins.py	2008-12-12 04:49:02 +0000
    aaron> @@ -801,15 +801,8 @@
    aaron>              result.report(self.outf)
    aaron>              if verbose and result.old_revid != result.new_revid:
    aaron> -                old_rh = list(
    aaron> -                    branch_to.repository.iter_reverse_revision_history(
    aaron> -                    result.old_revid))
    aaron> -                old_rh.reverse()
    aaron> -                new_rh = branch_to.revision_history()
    aaron> -                log_format = branch_to.get_config().log_format()
    aaron> -                log.show_changed_revisions(branch_to, old_rh, new_rh,
    aaron> -                                           to_file=self.outf,
    aaron> -                                           log_format=log_format)
    aaron> +                log.show_branch_change(branch_to, self.outf, result.old_revno,
    aaron> +                                       result.old_revid)

Why don't you respect the log_format parameter here ?

    aaron> +                
    aaron>          finally:
    aaron>              branch_to.unlock()

    aaron> === modified file 'bzrlib/log.py'
    aaron> --- bzrlib/log.py	2008-11-27 03:18:40 +0000
    aaron> +++ bzrlib/log.py	2008-12-12 02:43:42 +0000
    aaron> @@ -1004,6 +1004,101 @@
    aaron>                   search=None)
    aaron> +def get_history_change(old_revision_id, new_revision_id, repository):
    aaron> +    """Calculate the uncommon lefthand history between two revisions.
    aaron> +
    aaron> +    :param old_revision_id: The original revision id.
    aaron> +    :param new_revision_id: The new revision id.
    aaron> +    :param repository: The repository to use for the calcualtion.
    aaron> +
    aaron> +    return old_history, new_history
    aaron> +    """

Very nice one, but until we carefully rewrite and review log to
know what should be public or not, please make it private.

    aaron> +
    aaron> +def show_flat_log(repository, history, last_revno, lf):
    aaron> +    """Show a simple log of the specified history.
    aaron> +
    aaron> +    :param repository: The repository to retrieve revisions from.
    aaron> +    :param history: A list of revision_ids indicating the lefthand history.
    aaron> +    :param last_revno: The revno of the last revision_id in the history.
    aaron> +    :param lf: The log formatter to use.
    aaron> +    """
    aaron> +    start_revno = last_revno - len(history) + 1
    aaron> +    revisions = repository.get_revisions(history)
    aaron> +    for i, rev in enumerate(revisions):
    aaron> +        lr = LogRevision(rev, i + last_revno, 0, None)
    aaron> +        lf.log_revision(lr)
    aaron> +
    aaron> +

Clearly duplicate some code present in bzrlib.log.

Finally, show_changed_revisions() is also used by push, why not
upgrading it too ?



More information about the bazaar mailing list