-
Notifications
You must be signed in to change notification settings - Fork 17
consolidate to single file per package #12
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
Conversation
|
Pending to introduce some more test cases to protect against issues with circular dependencies of objects, and introduce logic to consistently order generated classes |
| @Value.Default | ||
| default String className() { | ||
| return ""; | ||
| return "__version__"; |
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.
This seems like an odd default override for something named PythonLine.
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.
ya, The abstraction is pretty broken imo.
| '--packageVersion', '0.0.0' | ||
|
|
||
| inputs.files configurations.verificationApi | ||
| doLast { delete "python/test/setup.py" } |
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 delete setup.py?
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.
because in this usage its just a submodule of test rather than a standalone module
|
+1 on adding integration tests for circle deps |
|
The revert is a little tough to confirm since some of the cleanup is reflected here (which is fine). Do we have a list of the other cleanup that will need to be redone? |
|
@ferozco, can you also provide more context on why this reverted? |
* Assert that ignored tests would have failed * this test is actually fine * Blacklist only particular path param tests
Revert change which introduced generating different classes/types/services in different files which broke recursive type definition. Now they are consolidated on a per package basis into a single
__init__.pyfile. Unfortunately, that means that some of the cleanup I had done also goes away, I'll address this in a follow up.