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

Updates README network section with better example #41

Merged
merged 4 commits into from
Feb 17, 2022

Conversation

jmgilman
Copy link
Contributor

Addresses #40 by making a small addendum in the network section example to show how to use the network name when creating a container.

@jmgilman
Copy link
Contributor Author

I took a crack at fixing the CI errors. Notably, I bumped the minimum Python version to 3.7.0 since 3.6 went EOL this year anyways.

@Jc2k
Copy link
Owner

Jc2k commented Feb 17, 2022

Thanks so much! Wasn't sure when I was gonna find the time for that 🤣 good call on 3.6.

@codecov
Copy link

codecov bot commented Feb 17, 2022

Codecov Report

Merging #41 (6f8ea99) into main (11eecfe) will increase coverage by 0.33%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #41      +/-   ##
==========================================
+ Coverage   93.05%   93.39%   +0.33%     
==========================================
  Files          18       18              
  Lines         547      560      +13     
==========================================
+ Hits          509      523      +14     
+ Misses         38       37       -1     
Flag Coverage Δ
unittests 93.39% <ø> (+0.33%) ⬆️

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

Impacted Files Coverage Δ
examples/resolver-service/conftest.py 100.00% <0.00%> (ø)
pytest_docker_tools/factories/build.py 100.00% <0.00%> (ø)
pytest_docker_tools/factories/fetch.py 100.00% <0.00%> (ø)
pytest_docker_tools/factories/image.py 100.00% <0.00%> (ø)
pytest_docker_tools/factories/volume.py 100.00% <0.00%> (ø)
pytest_docker_tools/factories/container.py 94.59% <0.00%> (+0.15%) ⬆️
pytest_docker_tools/plugin.py 70.37% <0.00%> (+1.13%) ⬆️
pytest_docker_tools/wrappers/container.py 83.33% <0.00%> (+1.44%) ⬆️

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 533f806...6f8ea99. Read the comment docs.

@Jc2k
Copy link
Owner

Jc2k commented Feb 17, 2022

I think we want to have pytest run in the root dir so that pytest-markdown runs for the README. Or maybe specifically pass the readme to pytest.

@jmgilman
Copy link
Contributor Author

Ah, that makes more sense. I couldn't figure out why it was hitting the README, didn't realize there was a plugin that was grabbing it.

@Jc2k
Copy link
Owner

Jc2k commented Feb 17, 2022

I'm really bad at testing my documentation so I thought i better run it as tests 😆

@jmgilman
Copy link
Contributor Author

Well turns out I'm just as bad since I didn't modify the imports on the example I expanded upon :) I think everything should pass now and I reverted pytest to start from the root again.

@Jc2k
Copy link
Owner

Jc2k commented Feb 17, 2022

Awesome! Thank you so much for sorting that out 😄

@Jc2k Jc2k merged commit e51d353 into Jc2k:main Feb 17, 2022
@jmgilman
Copy link
Contributor Author

No worries. If you can manage a release soon with the package updates that would be awesome :) I'm getting dependabot alerts on the docker package being used by the last release.

@jmgilman jmgilman deleted the issues/40 branch February 17, 2022 12:50
@Jc2k
Copy link
Owner

Jc2k commented Feb 17, 2022

I have done a release that unpins the docker version (3.1.3). But I don't think that will stop your alert, unfortunately:

docker/docker-py#2943

@Jc2k
Copy link
Owner

Jc2k commented Feb 17, 2022

Kinda looks like the official python docker client is unmaintained

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