Skip to content
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

Issue #131 - Pin jsonschema to 3.2.0 #132

Merged
merged 2 commits into from
Jan 13, 2022
Merged

Conversation

adammkelly
Copy link
Contributor

No description provided.

@adammkelly adammkelly changed the title Issue #131 - Pin jsonschema to 0.3.2 Issue #131 - Pin jsonschema to 3.2.0 Oct 1, 2021
@@ -1,4 +1,4 @@
jsonschema
jsonschema==3.2.0
Copy link

Choose a reason for hiding this comment

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

Maybe remove that dependency completely as is being required already by the openapi-schema-validator dependency (and also remove from setup.cfg)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's true but we use jsonschema directly also and version requirements for these two libraries can be different.

Copy link

@sthagen sthagen Jan 11, 2022

Choose a reason for hiding this comment

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

But pinning (==) on one side of two never solves such a different needs problem with pip - there must be intervals of versions so that pip can find a non-empty intersection with versions to choose from (as only a single version can be installed inside an environment.

Copy link
Collaborator

@p1c2u p1c2u Jan 11, 2022

Choose a reason for hiding this comment

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

I agree, this should be caret requirement ^3.2.0 before we move to 4.x

Copy link

@sthagen sthagen Jan 11, 2022

Choose a reason for hiding this comment

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

@p1c2u caret reads a bit npmish to me, wouldn't that be jsonschema~=3.2.0 for requirements.txt if we are afraid of upstream future versions 3.3 and larger? Update: Ah, poetry accepts that ("^3.2.0") also in pyproject.toml.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I plan to move the library to poetry, that's why

Copy link
Collaborator

@p1c2u p1c2u Jan 13, 2022

Choose a reason for hiding this comment

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

@sthagen this is just for requirements.txt which is used for testing environment. For setup.cfg I would set jsonschema<5.0.0to exclude future version with incompatible iter_errors.

@codecov
Copy link

codecov bot commented Jan 13, 2022

Codecov Report

Merging #132 (ca4d68f) into master (27ee7cf) will decrease coverage by 0.32%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #132      +/-   ##
==========================================
- Coverage   98.46%   98.14%   -0.33%     
==========================================
  Files          19       19              
  Lines         520      538      +18     
==========================================
+ Hits          512      528      +16     
- Misses          8       10       +2     
Impacted Files Coverage Δ
openapi_spec_validator/__main__.py 94.23% <0.00%> (-2.83%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27ee7cf...ca4d68f. Read the comment docs.

@p1c2u p1c2u merged commit f8645de into python-openapi:master Jan 13, 2022
Copy link

@sthagen sthagen left a comment

Choose a reason for hiding this comment

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

👍🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants