search

wolfswolke / aniworld_scraper

Aniworld.to mp4 scraper for animes.
MIT License
56 stars 15 forks source link

Some improvements #25

Closed lks-hrsch closed 10 months ago

lks-hrsch commented 10 months ago

Major rework

I needed to create an main.py because otherwise I get some import issues with the local modules.

wolfswolke commented 10 months ago

Great PR! Can you please fix these things so i can merge it? You are using "Anime-Name-Goes-Here" and "Name-Goes-Here", You removed the URL Value from start_app\setup_arguments but still pass it on which results in a error, You now need to start MAIN.py and not start_app.py (ReadMe.md) (You can also add yourself there under Special Thanks). Could you also please remove -i https://pip... from requirments as its not needed anymore with the Logger Lib?

lks-hrsch commented 10 months ago

@wolfswolke thanks for your feedback. I improved my PR and I am ready for another review from you.

wolfswolke commented 10 months ago

@lks-hrsch How are you testing your commits? I just tested it and i immediately get the Error: ERROR:root:Exception: local variable 'ddos_start_value' referenced before assignment This is probably because you import ddos_start_timer and not ddos_start_value. (Maybe a typo?) Also when i run it via cli it doesn't log anymore... I don't see a change in the logging Process so this is very confusing. (Probably a issue on my end)

Another Question. Why do you use . in front of your imports? Is there any big benefit from using Relative Imports? I also read that you need the init.py for ever folder which has a PY file because of that. In my eyes this can be done with normal imports and either a VENV or a clear Source Root.

Im not trying to say your Idea or code is wrong. Im just curios what the benefit of it is.

lks-hrsch commented 10 months ago

@wolfswolke sorry it was all my fault I wasn't testing that extensively. Now I have 2 runtime configurations in my pycharm one with CLI-Arguments and one without.

I had the same issue with the logging. Now it should work more reliable.

Another Question. Why do you use . in front of your imports?

It's my personal preference, I always use inside of packages relative imports and outside absolute. I changed it so it should be now normal.

wolfswolke commented 10 months ago

@lks-hrsch Thank you again! There is still a mix up with "Anime-Name-Goes-Here" and "Name-Goes-Here" (constants/Start_app 42) Can you please fix that and then i will merge the PR!

lks-hrsch commented 10 months ago

@wolfswolke now everything should be resolved or?