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

Asp/move http to common #109

Merged
merged 23 commits into from
Sep 14, 2023
Merged

Asp/move http to common #109

merged 23 commits into from
Sep 14, 2023

Conversation

alexespencer
Copy link
Contributor

For an upcoming change to kraken-wrapper that requires the use of http, the module sits better in kraken-common than kraken-std. For now, I've copied the module with the linting tests, and left the module in kraken-std but added a warning that it's deprecated

@alexespencer
Copy link
Contributor Author

Note: I did not write the http module, I've lifted it from kraken.std - there were previous comments on that PR about the function names, particularly within the linting section, but I've chosen to keep them as is, lift and shift.

@alexespencer alexespencer marked this pull request as ready for review September 13, 2023 13:26
Copy link
Contributor

@NiklasRosenstein NiklasRosenstein left a comment

Choose a reason for hiding this comment

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

Left two comments, but overall LGTM 🚀

kraken-common/pyproject.toml Outdated Show resolved Hide resolved
kraken-common/src/kraken/common/http/__init__.py Outdated Show resolved Hide resolved
@NiklasRosenstein NiklasRosenstein merged commit 0fc0b11 into develop Sep 14, 2023
13 checks passed
@NiklasRosenstein NiklasRosenstein deleted the asp/move-http-to-common branch September 14, 2023 14:39
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