Code review comment for lp:~mwhudson/loggerhead/bug-383631

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

On 1/8/2012 10:54 PM, Michael Hudson-Doyle wrote:
> Michael Hudson-Doyle has proposed merging
> lp:~mwhudson/loggerhead/bug-383631 into lp:loggerhead.
>
> Requested reviews: Loggerhead Reviewers (loggerhead-reviewers)
> Related bugs: Bug #383631 in loggerhead: "Breadcrumbs while
> annotating a file in head shows the most recent revision in the url
> instead of head" https://bugs.launchpad.net/loggerhead/+bug/383631
>
> For more details, see:
> https://code.launchpad.net/~mwhudson/loggerhead/bug-383631/+merge/87877
>
> This branch fixes the linked bug, twice over:
>
> 1) It changes the URLs of the in-branch breadcrumbs to be based on
> paths, like everything else in Loggerhead has been for years.
>
> 2) It removes 'file_id' from the set of query arguments that the
> (disgusting, I'd forgotten how horrible this bit was) machinery
> used by context_url preserves, now that we never ever generate a
> url with a file_id argument.
>
> Cheers, mwh

I love getting rid of file_id in the URLs, but will this break
existing URLs if people are saving them somewhere? ISTR some people
wanted a good way to point people at :head of a file like README, etc.

I especially like this:
- - 'file_id': inv.path2id('/'.join(dir_parts[:index + 1])),
+ 'path': '/'.join(dir_parts[:index + 1]),

Internally it was already doing paths, and forcing a translation to
file_id at this part.

Anyway, if it is incompatible, is there a way we could try to migrate
gracefully?

 review: needsinfo

John
=:->

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

iEYEARECAAYFAk8W3/UACgkQJdeBCYSNAAOPCQCfX+WJIFLOagJvmKJnLy+/tKXJ
snsAn0jlmnE6b3DHidXeMLPZLnOIUaP0
=UHk1
-----END PGP SIGNATURE-----