Noissue - Update virtualenv instructions

Noissue - Update virtualenv instructions

[Vasily Kuznetsov, 2019-01-25 12:09:50]

Hi Rosie,

Thanks for splitting this out. The changes look good but I have a couple of
thoughts about making our recommendations even better. Let me know if you agree.

Cheers,
Vasily

https://codereview.adblockplus.org/29989581/diff/29989582/README.rst
File README.rst (right):

https://codereview.adblockplus.org/29989581/diff/29989582/README.rst#newcode214
README.rst:214: Clone the repo to your local machine. Then create a virtualenv
and install
Since we have published the module on PyPI, we now also have an option of just
`pip install -U python-abp`. This might not work so well for developers of
python-abp or for those who would like to be on the bleeding edge, but in
general we should probably start recommending it as the primary way of using the
library.

https://codereview.adblockplus.org/29989581/diff/29989582/README.rst#newcode219
README.rst:219: $ source venv/bin/activate
Since we don't need the virtualenv to be activated to use the library with R (at
least it seems like it from the README), we could instead just do `venv/bin/pip
install --upgrade .`.

https://codereview.adblockplus.org/29989581/diff/29989582/README.rst#newcode235
README.rst:235: When you are finished, deactivate the virtualenv and delete it::
If you agree with the suggestion above about not activating the virtualenv, we
don't really need this part. Most likely people will keep using the library and
they would want to keep the virtualenv. The also won't need to deactivate it
because it was never activated.

If you prefer the activation approach, we should probably still consider the
likely use case of using the same installation of python-abp and the same
virtualenv many times. I would say that the part about deleting it should be
more separated.

[rhowell, 2019-01-25 23:49:00]

Hey Vasily,

Thanks for the suggestions. Does this look better?

https://codereview.adblockplus.org/29989581/diff/29989582/README.rst
File README.rst (right):

https://codereview.adblockplus.org/29989581/diff/29989582/README.rst#newcode214
README.rst:214: Clone the repo to your local machine. Then create a virtualenv
and install
On 2019/01/25 12:09:50, Vasily Kuznetsov wrote:
> Since we have published the module on PyPI, we now also have an option of just
> `pip install -U python-abp`. This might not work so well for developers of
> python-abp or for those who would like to be on the bleeding edge, but in
> general we should probably start recommending it as the primary way of using
the
> library.

Done.

https://codereview.adblockplus.org/29989581/diff/29989582/README.rst#newcode219
README.rst:219: $ source venv/bin/activate
On 2019/01/25 12:09:50, Vasily Kuznetsov wrote:
> Since we don't need the virtualenv to be activated to use the library with R
(at
> least it seems like it from the README), we could instead just do
`venv/bin/pip
> install --upgrade .`.

Done.

https://codereview.adblockplus.org/29989581/diff/29989582/README.rst#newcode235
README.rst:235: When you are finished, deactivate the virtualenv and delete it::
On 2019/01/25 12:09:50, Vasily Kuznetsov wrote:
> If you agree with the suggestion above about not activating the virtualenv, we
> don't really need this part. Most likely people will keep using the library
and
> they would want to keep the virtualenv. The also won't need to deactivate it
> because it was never activated.
> 
> If you prefer the activation approach, we should probably still consider the
> likely use case of using the same installation of python-abp and the same
> virtualenv many times. I would say that the part about deleting it should be
> more separated.

Done.

[Vasily Kuznetsov, 2019-01-28 16:26:34]

Hi Rosie!

Almost done, I just have one more nit.

Cheers,
Vasily

https://codereview.adblockplus.org/29989581/diff/29990566/README.rst
File README.rst (right):

https://codereview.adblockplus.org/29989581/diff/29990566/README.rst#newcode230
README.rst:230: > use_virtualenv("~/python-abp/env", required=TRUE)
Maybe we should add a comment (like # this kind of comment) saying that this
line is only needed if we use the virtualenv option from above.

[rhowell, 2019-01-29 00:35:48]

Ah, good catch!

https://codereview.adblockplus.org/29989581/diff/29990566/README.rst
File README.rst (right):

https://codereview.adblockplus.org/29989581/diff/29990566/README.rst#newcode230
README.rst:230: > use_virtualenv("~/python-abp/env", required=TRUE)
On 2019/01/28 16:26:34, Vasily Kuznetsov wrote:
> Maybe we should add a comment (like # this kind of comment) saying that this
> line is only needed if we use the virtualenv option from above.

Done.

[Vasily Kuznetsov, 2019-01-29 15:55:37]

LGTM