Code review comment for lp:~jml/bzr/lp-login-oauth-2

On Wed, Dec 16, 2009 at 6:53 PM, Martin Pool <email address hidden> wrote:
> Review: Needs Information
> needs a news entry
>
> 115     + except ImportError:
> 116     + raise BzrCommandError(
> 117     + "%s requires launchpadlib" % self.name())
> 118     + return lp_api
>
> I suspect this is duplicated but it's tolerable.
>

Cool.

> 178     +
> 179     +# XXX: Not the right value for Windows
> 180     +CACHE_DIRECTORY = os.path.expanduser('~/.launchpadlib/cache')
>
>
> 217     + credential_name = 'creds-%s-bzr' % (web_root_uri.host)
> 218     + return os.path.join(CACHE_DIRECTORY, credential_name)
>
> I thought you were going to tweak that?  Did you forget to push?  I think it would be worth changing it because it's going to create persistent data, and there are win32utils things to get the right path.  On Unix I'd like it to be either xdg compliant or inside ~/.bazaar.
>

I did tweak it, you are looking at an out-of-date diff, methinks.

> Also, are you sure lplib save_to() will take care of setting the permissions?

No, it won't. Again, I think you are looking at an out-of-date diff.

I'll add a NEWS entry.

jml