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

update isort and black #251

Merged
merged 7 commits into from Nov 9, 2020
Merged

update isort and black #251

merged 7 commits into from Nov 9, 2020

Conversation

Chilipp
Copy link
Contributor

@Chilipp Chilipp commented Nov 8, 2020

Hey @koxudaxi!

very nice package, thanks a lot for your work on this! I'd like to use it and I am already using the latest version of isort and black. It would be nice if the datamodel-code-generator is compatible with this which is whiy I make this PR.

From my perspective, this PR can be merged if the CI passes.

@Chilipp
Copy link
Contributor Author

Chilipp commented Nov 8, 2020

hmm ok. the CI is failing because a new version of black is installed. But to me, this is actually rather a CI issue and does not justify to force the users of this package to install an old version of black.

not sure what would be the best to fix this. One option would be to create a venv in lint.sh to make sure that the correct version of black and isort is installed

#!/usr/bin/env bash

set -e

python -m venv lint
source lint/bin/activate

pip install black==19.10b0 "isort>=4.3.21,<5.0"

black --check datamodel_code_generator tests
isort --check-only datamodel_code_generator tests

deactivate
rm -r lint

mypy datamodel_code_generator

what do you think @koxudaxi?

@koxudaxi
Copy link
Owner

koxudaxi commented Nov 9, 2020

@Chilipp
Thank you for creating this issue.
I understand your suggestion.
But, some users want to keep the old version of dependence packages.
we talked about the problem.
#195
Can you support the old version too?

I have checked your PR.
A related code of isort is changed.
I would we can check the version before run sort.

if isort.__version__.startswith('4.'): 
   return SortImports(file_contents=code).output
else:
   return isort.code(code) 

not sure what would be the best to fix this. One option would be to create a venv in lint.sh to make sure that the correct version of black and isort is installed

I think the change is good 👍

@Chilipp
Copy link
Contributor Author

Chilipp commented Nov 9, 2020

I would we can check the version before run sort.

alright, done with cc25f69

I think the change is good +1

ok, see 6a2397c

don't know why this dropped out...
@Chilipp
Copy link
Contributor Author

Chilipp commented Nov 9, 2020

alright, the same problem now with test.sh and test.bat. I'd fix this with something like

#!/usr/bin/env bash
set -e

python -m venv venv

source venv/bin/activate
pip install -e .[all] isort==4.3.21 "black>=19.10b0,<20"

pytest --cov=datamodel_code_generator --cov-report term-missing tests

deactivate
rm -r venv

what do you think @koxudaxi ?

@koxudaxi
Copy link
Owner

koxudaxi commented Nov 9, 2020

@Chilipp
I think your approach is good 🚀

@codecov
Copy link

codecov bot commented Nov 9, 2020

Codecov Report

Merging #251 (849955a) into master (e9b6edf) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #251   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        12    +1     
  Lines         1020      1082   +62     
  Branches       201       210    +9     
=========================================
+ Hits          1020      1082   +62     
Flag Coverage Δ
unittests 100.00% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...tamodel_code_generator/model/pydantic/dataclass.py 100.00% <ø> (ø)
datamodel_code_generator/__main__.py 100.00% <100.00%> (ø)
datamodel_code_generator/model/base.py 100.00% <100.00%> (ø)
...amodel_code_generator/model/pydantic/base_model.py 100.00% <100.00%> (ø)
..._code_generator/model/pydantic/custom_root_type.py 100.00% <100.00%> (ø)
datamodel_code_generator/model/pydantic/imports.py 100.00% <100.00%> (ø)
datamodel_code_generator/model/pydantic/types.py 100.00% <100.00%> (ø)
datamodel_code_generator/parser/base.py 100.00% <100.00%> (ø)
datamodel_code_generator/parser/jsonschema.py 100.00% <100.00%> (ø)
datamodel_code_generator/parser/openapi.py 100.00% <100.00%> (ø)
... and 3 more

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 bddf61a...849955a. Read the comment docs.

@Chilipp
Copy link
Contributor Author

Chilipp commented Nov 9, 2020

alright, seems to work @koxudaxi 😄 I am done here, merge it if you like

Thanks a lot again for your work on this =)

@koxudaxi
Copy link
Owner

koxudaxi commented Nov 9, 2020

I just came up with a good approach.
If list.sh have to change arguments of isort by versions, we can check the isort version in the shell scripts and use the if statement.

I may change the shell scripts.

Thank you very much.

@Chilipp
Copy link
Contributor Author

Chilipp commented Nov 9, 2020

sorry, I can't really follow, but do what you think is the best @koxudaxi 😅

@koxudaxi koxudaxi merged commit 8ed4841 into koxudaxi:master Nov 9, 2020
@koxudaxi
Copy link
Owner

koxudaxi commented Nov 9, 2020

No problem. I was too late getting the idea 😅
I will release tonight:rocket:

@Chilipp
Copy link
Contributor Author

Chilipp commented Nov 9, 2020

awesome! thanks a lot =)

@koxudaxi
Copy link
Owner

koxudaxi commented Nov 9, 2020

@Chilipp
I have released a new version as 0.6.3

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.

None yet

2 participants