-
Notifications
You must be signed in to change notification settings - Fork 0
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
PLATFORM-366 Fix mypy structural errors for library #97
PLATFORM-366 Fix mypy structural errors for library #97
Conversation
1a9c59a
to
15a926f
Compare
@@ -30,3 +31,9 @@ select = ['E', 'W', 'F', 'C', 'N'] | |||
ignore = ['E501'] | |||
line-length = 95 | |||
exclude = ["ca_cdk_constructs/eks/imports"] | |||
|
|||
[[tool.mypy.overrides]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the double square brackets?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, that was the example from mypy. Not sure what it's doing in pyproject syntax, but I'll have to have a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the TOML table array syntax. It means you can have multiple [[tool.mypy.overrides]]
elements, each one setting a different config for different packages.
15a926f
to
0c23cb3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
0c23cb3
to
3b01d88
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, sorry it took so long, I just saw it
Describe your changes
Testing using a git reference to this branch and importing into eks-platform-cdk. This removes the errors from the library being structured badly.
The change will all the null coalescing could have been done another way. The problem was that the argument was being passed directly from a function with optional arguments and a default to a function which required a non-null argument. This change doesn't affect the interface at all, so it's the one I've gone with, but one of the other alternatives of changing the function definitions might be the way forward in the long run..
Checklist before requesting a review