Skip to content

Add eachdist.py to simplify build. #291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Dec 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
afc7722
Add eachdist.py to simplify build.
Oberon00 Nov 15, 2019
56f2fb3
Fix coverage build.
Oberon00 Nov 15, 2019
4efd2af
Mark eachdist.py executable.
Oberon00 Nov 15, 2019
e5dcf63
Fix eachdist.py for Python 3.4 & 3.5
Oberon00 Nov 15, 2019
c38b2fb
Fix lint.
Oberon00 Nov 15, 2019
e62f5bc
Fix linting missing files.
Oberon00 Nov 15, 2019
2524efe
Fix black ignoring files on travis.
Oberon00 Nov 15, 2019
e3266e7
Merge branch 'master' into eachdist-script
Oberon00 Nov 18, 2019
7693a40
Merge branch 'master' into eachdist-script
Oberon00 Nov 22, 2019
ea07bd6
Add extraroots to cover missing py files.
Oberon00 Nov 22, 2019
ef7dd33
eachdist.py: Add --eager-upgrades, default w/ develop.
Oberon00 Nov 22, 2019
51f5d8d
Fix lint.
Oberon00 Nov 22, 2019
945e1e7
Merge branch 'master' into eachdist-script
Oberon00 Nov 25, 2019
83120d6
Make sure testutil & wsgi are installed before dependents.
Oberon00 Nov 25, 2019
66e9c8a
Fix dependencies for ext-testutil.
Oberon00 Nov 25, 2019
f216836
Merge branch 'master' into eachdist-script
Oberon00 Nov 26, 2019
cf7e685
Fix wrong dependency order for tracecontext in tox.ini.
Oberon00 Nov 26, 2019
10cfb56
Merge branch 'master' into eachdist-script
Oberon00 Dec 5, 2019
c5a2aa6
Fix mypy version being ignored.
Oberon00 Dec 5, 2019
4052f5f
Fix error message when calling without command.
Oberon00 Dec 6, 2019
487a600
eachdist.py: Document exec, improve doc in general.
Oberon00 Dec 6, 2019
3c203be
Typo.
Oberon00 Dec 6, 2019
a8800b2
Merge branch 'master' into eachdist-script
Oberon00 Dec 9, 2019
ac4174b
Lint jaeger examples.
Oberon00 Dec 9, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ exclude =
.svn
.tox
CVS
.venv*/
venv*/
target
__pycache__
ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/gen/
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ lib
lib64
__pycache__
venv*/
.venv*/

# Installer logs
pip-log.txt
Expand Down
2 changes: 1 addition & 1 deletion .isort.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ line_length=79
; docs: https://github.com/timothycrosley/isort#multi-line-output-modes
multi_line_output=3
skip=target
skip_glob=ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/gen/*
skip_glob=ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/gen/*,.venv*/*,venv*/*
known_first_party=opentelemetry
9 changes: 8 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@ during their normal contribution hours.

## Development

This project uses [`tox`](https://tox.readthedocs.io) to automate some aspects
To quickly get up and running, you can use the `scripts/eachdist.py` tool that
ships with this project. First create a virtualenv and activate it.
Then run `python scripts/eachdist.py develop` to install all required packages
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between python scripts/eachdist.py develop and python scripts/eachdist.py install --editable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an alias for install --editable --with-dev-deps --eager-upgrades (see

editable=True, with_dev_deps=True, eager_upgrades=True
)

as well as the project's packages themselves (in `--editable` mode).
You can then run `scripts/eachdist.py test` to test everything or
`scripts/eachdist.py lint` to lint everything (fixing anything that is auto-fixable).

Additionally, this project uses [`tox`](https://tox.readthedocs.io) to automate some aspects
of development, including testing against multiple Python versions.

You can run:
Expand Down
10 changes: 10 additions & 0 deletions dev-requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
pylint~=2.3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this file have the word constraints in it as well? dev-constraints.txt maybe? Due to use using it as constraints in tox.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, if you use the install --with-dev-deps or develop commands, it will be used as a requirements file.

flake8~=3.7
isort~=4.3
black>=19.3b0,==19.*
mypy==0.740
sphinx~=2.1
sphinx-rtd-theme~=0.4
sphinx-autodoc-typehints~=1.10.2
pytest!=5.2.3
pytest-cov>=2.8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand these were moved from tox.ini, but what do you think about using the latest version of build tools instead of pinning these? It does mean dealing with some build unpredictability, but prevents us having to make big changes to update these packages later.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should at least have >= requirements (although the minimum version would be very low due to Python 3.4 support most of the time, at least for requirements we use on Python != 3.7). I'm fine with removing upper bounds though.

16 changes: 16 additions & 0 deletions eachdist.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# These will be sorted first in that order.
# All packages that are depended upon by others should be listed here.
[DEFAULT]
sortfirst=
opentelemetry-api
opentelemetry-sdk
ext/opentelemetry-ext-wsgi
ext/*

[lintroots]
extraroots=examples/*,scripts/
subglob=*.py,tests/,test/,src/*,examples/*

[testroots]
extraroots=examples/*,tests/
subglob=tests/,test/
2 changes: 1 addition & 1 deletion ext/opentelemetry-ext-testutil/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ package_dir=
=src
packages=find_namespace:
install_requires =
opentelemetry-ext-wsgi
opentelemetry-api

[options.extras_require]
test = flask~=1.0
Expand Down
16 changes: 1 addition & 15 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,7 @@ line-length = 79
exclude = '''
(
/(
Copy link
Contributor

@lzchen lzchen Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, this file is for excluding certain folders from being checked with black. We are removing these because the tox will run the eachdist.py script (which will only run on folders specified in eachdist.ini), instead of running black directly correct? For cases when I want to fix the lint directly myself without running tox, will changing this file effect that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, running black directly vs eachdist.py makes no difference at all. This was removed because black already checks .gitignore and so the lines were just redundant. This is actually a change independent from the rest of this PR and could be merged separately.

\.eggs # exclude a few common directories in the
| \.git # root of the project
| \.hg
| \.mypy_cache
| \.tox
| \.venv
| \.vscode
| _build
| buck-out
| target
| build
| dist
| ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/gen # generated files
ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/gen # generated files
)/
| foo.py # also separately exclude a file named foo.py in
# the root of the project
)
'''
Loading