Code review comment for lp:~maxb/launchpad/bug-497731

Revision history for this message
Guilherme Salgado (salgado) wrote :

<salgado> no, just remove get_BZR_PLUGIN_PATH_for_subprocess() and change get_bzr_plugins_path() to append a "-site" to its return value
<maxb> That's not acceptable because the bare directory is required for the load_plugins([...]) call ~10 lines below
<salgado> oh, I missed that
<salgado> why is it required there?
<maxb> The way it works is that if you're calling that bzrlib call directly, bzrlib uses *exactly* the directories you pass it. If you're setting the environment variable, bzrlib augments it with the standard directories unless you use the magic tokens to tell it not to
<salgado> I see
<salgado> maxb, then we can rename the existing one to _get_bzr_plugins_path() and remove it from __all__
<maxb> Sounds good
<salgado> maxb, also, it'd be nice to state in the new docstring why we use the "-site" magic token

review: Approve (code)

« Back to merge proposal