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

Move config parameters from class to instance #20 #51

Merged
merged 3 commits into from
Oct 14, 2022

Conversation

svenXY
Copy link

@svenXY svenXY commented Oct 13, 2022

Implements a possible solution for #20

@svenXY
Copy link
Author

svenXY commented Oct 13, 2022

OK, this does not work. It seems that constants changed in the subclass are somehow lost.

You are right - the problem is how to implement the new interface while keeping the old one as they contradict each other

@svenXY
Copy link
Author

svenXY commented Oct 13, 2022

OK, the current version passes all tests. Maybe you could restart the test suites on your side?

@andrii-bodnar
Copy link
Member

@svenXY something wrong, the checks have failed again

@svenXY
Copy link
Author

svenXY commented Oct 14, 2022

@andrii-bodnar
Copy link
Member

@svenXY hmm, do you have any ideas on how to fix that? We need to support at least 3.6+ Python versions

@svenXY
Copy link
Author

svenXY commented Oct 14, 2022

Should work now. But honestly, maybe it is really better to drop the old approach with class constants as that would also mean that we could drop most of the Optional[]s in __init__()

@svenXY
Copy link
Author

svenXY commented Oct 14, 2022

this last failure is something else and has afaiu nothing to do with my code:

'{'detail': ErrorDetail(string='Unable to locate build via Github Actions API. Please upload with the Codecov repository upload token to resolve issue.', code='not_found')}'

@andrii-bodnar
Copy link
Member

@svenXY Thank you!

Yes, it's something not related to your changes. Will try to investigate that

@andrii-bodnar andrii-bodnar linked an issue Oct 14, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move config parameters from class to instance
3 participants