Remove eachdist from documentation #1995
Conversation
ocelotl
requested review from
a team,
owais and
codeboten
and removed request for
a team
| @@ -45,18 +45,11 @@ during their normal contribution hours. | |||
|
|
|||
| ## Development | |||
|
|
|||
| To quickly get up and running, you can use the `scripts/eachdist.py` tool that | |||
There was a problem hiding this comment.
Please document how to achieve this with tox or otherwise (e.g. I'd like to be able to go-to-definition across projects within VS code/Pylance)
There was a problem hiding this comment.
Sorry, @Oberon00 I don't understand the following:
(e.g. I'd like to be able to go-to-definition across projects within VS code/Pylance)
Can you please give me more context? ✌️
There was a problem hiding this comment.
If I just clone the repository, create an empty virtualenv and open the folder in VS Code, it will not be able to resolve an import of, e.g., Span from opentelemetry-api while I'm editing a file in opentelemetry-sdk.
If I do pip install -e ./opentelemetry-api, it will be able to resolve the import.
So one way to do this without eachdist.py is of course to pip install -e everything, or everything I'm interested in, manually, in the right order (dependencies before depending projects).
But it would probably possible to replace this with tox somehow. E.g. I saw there is a recent feature with a promising name https://tox.readthedocs.io/en/latest/example/devenv.html.
But what we would need is a tox environment that does a pip install -e for each distribution in the project (I think that's where I got eachdist.py's name from originally).
EDIT: Or maybe I'm the only one that thinks this is useful, and there would be a better workflow I'm not aware of?
There was a problem hiding this comment.
We could maybe have a tox ennvironment called (pyxx-)devenv that just calls eachdist.py develop {posargs} behind the scenes for now. This could be transparently replaced with a solution that does not use the script (I have stated my personal opinion on that removal in #1462, but assuming you want that)
There was a problem hiding this comment.
Every time you run tox, virtual environments are created in the .tox folder and you can activate them, for example, for tox -e lint: source .tox/lint/bin/activate.
There was a problem hiding this comment.
I get that, but I was asking was whether there is
a tox environment that does a pip install -e for each distribution in the project
Looking at the toxfile now, the lint environment seems to do this. Could we mention that here?
Like
To quickly get up and running, you can run
tox -e lintand then activate the resulting virtual environment with. .tox/lint/bin/activate. This has all distributions in the repository installed in editable mode, as well as the black autoformatter and the pylint and flake8 linting tools.
There was a problem hiding this comment.
Of course, the "quickly" part is hapered a bit by the fact that this environment will run the very slow pylint, so maybe a dedicated environment would still be better.
There was a problem hiding this comment.
Ok, I understand now what you mean. I think that is outside of the scope of this PR, adding that feature to tox.ini would be better tracked in a separate issue.
There was a problem hiding this comment.
Make sense, but maybe suggesting the environment created by tox -e lint would still be better than nothing.
There was a problem hiding this comment.
It looks like with that devenv feature @Oberon00 shared, you could just update the instructions to say tox --devenv venv -e lint and that prevents the commands from running:
| to run tests. `git clone [email protected]:open-telemetry/opentelemetry-python-contrib.git opentelemetry-python-contrib`. | ||
|
|
||
| You can then run `scripts/eachdist.py test` to test everything or |
There was a problem hiding this comment.
Is it possible to run tox without linting (only tests)? For me, the pylint step takes probably longer than everything else together.
There was a problem hiding this comment.
Yes, tox -e py39-test-core-api, for example.
There was a problem hiding this comment.
OK, but I guess there is no way to run all tests without either listing them one by one or also running lint, etc at the same time. Not a must-have feature though.
EDIT: something like tox -e py39-test-*, but I don't think tox supports that kind of patterns.
| `tox -e lint` checks the style of the code. It uses [`black`](https://black.readthedocs.io/en/stable/), [`isort`](https://pycqa.github.io/isort/) | ||
| and [`pylint`](http://pylint.pycqa.org/en/latest/) for this purpose. `black` and `isort`-related errors can be fixed automatically by following | ||
| the next steps: | ||
|
|
||
| 1. `tox -e lint` | ||
| 2. `source .tox/lint/bin/activate` | ||
| 3. `black .` | ||
| 4. `isort .` | ||
| 5. `deactivate` |
There was a problem hiding this comment.
We should just create a new tox env to do this, so users can simply run tox -e fix.
|
I don't mind removing it from docs and relying completely on tox but just wanted to say that eachdist script was very nice when working with custom venvs outside tox. It'd be nice to keep it around even if undocumented. |
|
This PR is pretty outdated now, closing. |
Fixes #1994