refactor: new /clients directory - #399
Conversation
64bd7fa to
3a7f637
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Coverage Report for CI Build 30268537840Coverage increased (+0.04%) to 93.238%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
irstavr
left a comment
irstavr
left a comment
There was a problem hiding this comment.
There are 3 files here that only relate to linting issues. We could split them up in a different PR to help with the review?!
|
Sure! Let me chedrry pick them |
I plan on merging the linting errors in this PR #400 before I remove them from here. Sounds good? |
As part of #240, this change makes way for a new /clients directory. that is where the new base class for clients and the async implementation of the client will live.
There are not (and should not be) mentions of these elements outside of the module where they are declared.
0f100bb to
2b18e8a
Compare
sighphyre
left a comment
sighphyre
left a comment
There was a problem hiding this comment.
Failing check on the API token isn't new so I don't think that's a problem with this PR - don't think Github's suggestion is a good one though, this can probably either take a hash of the token or trumcate is to 6 characters after the project, so turn something like production-env:default-project:a7d651bae8aaefdb9179 -> production-env:default-project:a7d651, enough to not make it useful but still enough to make it recognizable. Can chat on Slack if you need
I very much don't agree with collapsing the environment_resolver into utils, because I think utils is where good code structure goes to die. That being said, this is a taste thing and not a blocker.
Structure is nice, change makes sense, looks good! Happy if you merge this
9eed8a8 to
58a3914
Compare
6 participants
Summary
Making way for what's to come (yak shaving a bit), this PR includes two tiny rearrangements:
extract_environment_from_headersto utils.pyThis one is highly based on my opinion. The environment function seems to fit quite well inside of utils.py and that would be one less file to maintain.
/clientsdirectoryThis one prepares the directory structure for the implementation of #240, mostly to avoid conflict hells further down the line and to not mix more than one change in the same PR.