Code review comment for lp:~jr/bzr/314314-obsolete-packs

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

On 10/19/2011 3:42 PM, Jonathan Riddell wrote:
> Jonathan Riddell has proposed merging
> lp:~jr/bzr/314314-obsolete-packs into lp:bzr.
>
> Requested reviews: bzr-core (bzr-core) Related bugs: Bug #314314 in
> Bazaar: "bzr does not automatically create 'obsolete_packs' when it
> is missing." https://bugs.launchpad.net/bzr/+bug/314314
>
> For more details, see:
> https://code.launchpad.net/~jr/bzr/314314-obsolete-packs/+merge/79816
>
> * Create obsolete_packs directory when repacking if it does not
> exist. (Jonathan Riddell, #314314)
>

I really doubt this is correct:
@@ -1221,6 +1222,10 @@
         """
         for pack in packs:
             try:
+ os.mkdir("../obsolete_packs/")
+ except OSError:
+ pass
+ try:
                 pack.pack_transport.move(pack.file_name(),
                     '../obsolete_packs/' + pack.file_name())

Given that the following code is using "pack.pack_transport" that
indicates that this is handling Remote objects. I really don't want to
see "obsolete_packs" suddenly show up in my working directory after
doing 'bzr push'.

 review: needsfixing

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

iEYEARECAAYFAk6e2qoACgkQJdeBCYSNAAPBdgCcDEEfUEJBp+0Kilbk0vq9Afkq
0wAAn2nnPqtBvgWvGGAbiXKj6qAH3PK5
=qwd8
-----END PGP SIGNATURE-----