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

Cleans the kicost/distributors/distributor.py code #439

Merged
merged 1 commit into from
Mar 10, 2021

Conversation

set-soft
Copy link
Collaborator

@set-soft set-soft commented Mar 9, 2021

  • Remove unused imports
  • Fixed comments and indentation
  • Replaced distributors_modules (which no longer exists) by
    list(distributors_modules_dict) (the closest I can find).
    I think the get_dist_parts_info is never used and should be
    removed, or at least commented out.

- Remove unused imports
- Fixed comments and indentation
- Replaced `distributors_modules` (which no longer exists) by
  `list(distributors_modules_dict)` (the closest I can find).
I think the `get_dist_parts_info` is never used and should be
removed, or at least commented out.
@set-soft
Copy link
Collaborator Author

set-soft commented Mar 9, 2021

BTW: Which flake8 config should we use?

The code really needs a flake8 clean-up. I partially cleaned this source because I want to replace CurrencyConverter, and it was supposed to be used here (not really)

@hildogjr
Copy link
Owner

hildogjr commented Mar 9, 2021

Yes, about the flake8, I lot of legacy there. Much of than mine, before I really understood the PEP8. Use the standard configurations.

CurrencyConverter was I workaround that I added to currency conversion, something that was intrinsic made by the scrape (because of the website language).

Are you working on something for #386?

@set-soft
Copy link
Collaborator Author

set-soft commented Mar 9, 2021

Yes, about the flake8, I lot of legacy there. Much of than mine, before I really understood the PEP8. Use the standard configurations.

The main problem will be with 80 columns limitation. But I can cut lines to 80 columns. Today 80 columns is somehow ridiculous, even a Linux console is far wider on modern hardware.
BTW: you don't need to know much about PEP8, just let flake8 insult your style and fix what it says ;-) I use it for various projects, I just relax some details.

CurrencyConverter was I workaround that I added to currency conversion, something that was intrinsic made by the scrape (because of the website language).

Ok. I want to just remove the dependency. The functionality needed by KiCost is quite simpler (currency conversion for today, not historic) and this is poorly provided by the CurrencyConversion class.

Are you working on something for #386?

I'm currently trying to make a Linux GNU/Debian package containing KiCost. This will make easier the installation on Debian, Ubuntu and any derivative distro.

My main goal is to integrate KiCost with KiBot

@hildogjr
Copy link
Owner

hildogjr commented Mar 9, 2021

Fell free to disable the 80 columns, I also think it is bad and just create warning on actual displays.

I will check some of those commits and after release at PyPI.

@hildogjr hildogjr merged commit 9746edb into hildogjr:master Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants