search

fabric8-analytics / fabric8-analytics-license-analysis

License Analysis
GNU General Public License v3.0
6 stars 25 forks source link

[Technical debt] Avoid changing python path and write proper setup.py #14

Open pkajaba opened 7 years ago

pkajaba commented 7 years ago

Here is changed Python path and it's considered for bad practice:

https://github.com/fabric8-analytics/fabric8-analytics-license-analysis/blob/master/scripts/entrypoint.sh#L4

pkajaba commented 7 years ago

cc: @harjinder-hari

sara-02 commented 7 years ago

@pkajaba how to fix this?

pkajaba commented 7 years ago

@sara-02 you should remove where you mangle with python path, but it will stop working...

1) remove python path mangling 2) fix errors which were caused by this change.

jpopelka commented 7 years ago

it's considered for bad practice:

Do you have any pointers to support your claim ?

remove python path mangling

You mean the --pythonpath /src/ ?

pkajaba commented 7 years ago

Do you have any pointers to support your claim?

I can give you a reason why not to do in this case. I might not be a bad idea to mangle PYTHONPATH in some cases but it's not really necessary in case of this project.

First of all, there is no setup.py, so there is no proper way how to install it through pip. That's why we copy it to root folder of a container and then change python path.

I have to apologize since I did not phrase it correctly. Changing python path, in this case, is hacking and completely unnecessary. We don't have to fix right away since it does not affect functionaly, but it's technical debt.

You mean the --pythonpath /src/ ?

Yes :-)

EDIT: I changed the title and added a label. Hopefully, it makes more sense now.

tisnik commented 6 years ago

@sara-02 are you planning to add this issue to Planner and to the current sprint?