Bazaar

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

  1. lp-login-oauth-2
  2. Merge into bzr.dev
Revision history for this message
Martin Pool (mbp) wrote :

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.

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.

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

review: Needs Information

« Back to merge proposal