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

Add docker support #112

Merged
merged 9 commits into from
Feb 8, 2023
Merged

Add docker support #112

merged 9 commits into from
Feb 8, 2023

Conversation

opis-mark
Copy link
Contributor

Description

Add docker files to eliminate the installation and dependency requirements to have a working environment for tokendito.

Motivation and Context

I use containers for all of my python applications to avoid managing multiple python versions, dependencies, and virtual environments.

How Has This Been Tested?

I tested building and running the application with expected results.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@opis-mark opis-mark changed the title Feature/docker Add docker support Feb 4, 2023
@pcmxgti pcmxgti requested review from sevignyj and removed request for sevignyj February 6, 2023 15:17
@opis-mark
Copy link
Contributor Author

@sevignyj - If I'm reading the error correctly, the linting is upset about 3 files that I didn't change in this branch but yet the build succeeds in main which it is based on. Can you provide any insight?

@opis-mark
Copy link
Contributor Author

It looks like the code coverage report is showing a decrease in coverage on files that were not modified by this PR.

.dockerignore Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@sevignyj
Copy link
Contributor

sevignyj commented Feb 8, 2023

It looks like the code coverage report is showing a decrease in coverage on files that were not modified by this PR.

Hi @opis-mark , this is due to the PR being from the fork. We increased the threshold in coveralls to address that, your next push should not trigger a failure. In the future we will change the test coverages to not run on non-code change.

Really awesome PR, thank you so much @opis-mark !

Copy link
Contributor

@sevignyj sevignyj left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you @opis-mark

@sevignyj sevignyj merged commit 1c5d5e7 into dowjones:main Feb 8, 2023
@opis-mark opis-mark deleted the feature/docker branch February 9, 2023 00:46
@pcmxgti pcmxgti mentioned this pull request Feb 23, 2023
9 tasks
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