Skip to content

Check that API package installs #34

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.

Sign up for GitHub

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

Closed
c24t wants to merge 11 commits into from
Closed

Check that API package installs #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
493d179
Add mypy config
c24t Jun 26, 2019
5c26a0c
Fix various strict mypy errors
c24t Jun 26, 2019
dfe87df
Add tox mypy command
c24t Jun 26, 2019
9ff0101
Fix typo, bring back disallow_any_expr
c24t Jun 26, 2019
15b30bc
Require typing for old python versions
c24t Jun 26, 2019
9e5757e
Ignore contextmanager type errors
c24t Jun 26, 2019
a8e416d
Move toxfile
c24t Jun 26, 2019
085dc48
Move .pylintrc
c24t Jun 26, 2019
bd81da1
Try installing trace in all python versions
c24t Jun 27, 2019
b2393dd
Merge branch 'master' into check-install
c24t Jun 27, 2019
4870a2a
Merge branch 'master' into check-install
c24t Jun 27, 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: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ install:
- pip install tox-travis

script:
- tox
- (cd opentelemetry-api; tox)
Copy link
Contributor

@lzchen lzchen Jul 7, 2019

Choose a reason for hiding this comment

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

Just wondering, is this part of the WIP or are we only building what is inside the api package?

Copy link
Member Author

@c24t c24t Jul 9, 2019

Choose a reason for hiding this comment

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

We'll eventually build everything, for now only API has code to build.


branches:
only:
Expand Down
0 .pylintrc → opentelemetry-api/.pylintrc
View file
Open in desktop
File renamed without changes.
14 changes: 14 additions & 0 deletions opentelemetry-api/tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[tox]
Copy link
Member

@reyang reyang Jun 27, 2019

Choose a reason for hiding this comment

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

Curious, given we will have other packages (e.g. opentelemetry-sdk) under this repo, do we want to have a single global tox, or a tox file per package?

Copy link
Member

@Oberon00 Oberon00 Jun 27, 2019

Choose a reason for hiding this comment

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

I don't know what best practises are here, but I'd imagine we will have quite some code/config duplication with multiple toxfiles 🤔 I think OpenCensus-python used nox, maybe that tool is better in this regard?

Copy link
Member Author

@c24t c24t Jun 27, 2019

Choose a reason for hiding this comment

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

From a conversation with @reyang: nox gives us a combined coverage report if we have branches that depend on python version, but tox made it easier to run tests in parallel on travis. We'll stick with tox and burn the coverage bridge when we come to it.

I don't know whether it'll be easier to have one or multiple tox files. It does make it easier now to have it under opentelemetry-api since there's no code yet to check in the SDK.

skipsdist = True
envlist = py{34,35,36,37}-unit, py37-lint, py37-mypy

[testenv]
deps =
py{34,35,36,37}-unit: -e .
py37-lint: pylint
py37-mypy: mypy

commands =
py{34,35,36,37}-unit: python -c "from opentelemetry import trace"
py37-lint: pylint opentelemetry/
py37-mypy: mypy opentelemetry/
4 changes: 3 additions & 1 deletion tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
[tox]
skipsdist = True
envlist = py37-lint, py37-mypy
envlist = py{34,35,36,37}-unit, py37-lint, py37-mypy
Copy link
Contributor

@lzchen lzchen Jul 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

If there is going to be multiple tox files, is the idea to have a different build per package? I think we need to define what makes sense in the project structure to "build differently".

Copy link
Member Author

@c24t c24t Jul 9, 2019

Choose a reason for hiding this comment

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

Still TBD, I don't have an opinion on this.


[testenv]
deps =
py{34,35,36,37}-unit: -e .
py37-lint: pylint
py37-mypy: mypy

commands =
py{34,35,36,37}-unit: python -c "from opentelemetry import trace"
py37-lint: pylint opentelemetry-api/opentelemetry/
py37-mypy: mypy opentelemetry-api/opentelemetry/