-
Notifications
You must be signed in to change notification settings - Fork 85
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
Refactor the BasePipeline, move out all Project related logic #1351 #1358
Conversation
Signed-off-by: tdruez <[email protected]>
Signed-off-by: tdruez <[email protected]>
Signed-off-by: tdruez <[email protected]>
Signed-off-by: tdruez <[email protected]>
Signed-off-by: tdruez <[email protected]>
@keshav-space could you have a look before the merge? |
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.
Thanks @tdruez, Looking Good!
### Install | ||
|
||
```bash | ||
pip install aboutcode_pipeline |
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.
As per PEP8 https://peps.python.org/pep-0008/#package-and-module-names aboutcode-pipeline
would be a much more Pythonic package name.
Actually, going by @pombredanne suggestion here #1332 (comment), we may want to name this package aboutcode.pipeline
.
pip install aboutcode_pipeline | |
pip install aboutcode.pipeline |
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.
I initially went with aboutcode_pipeline in the README as the filenames of the flot build were aboutcode_pipeline-0.1.whl
, even though the [project] name was set to "aboutcode.pipeline".
Refactor the BasePipeline to remove the scanpipe.Project logic and make it reusable without Project context.