HarshCasper / Rotten-Scripts

Scripts that will make you go WOW!
MIT License
1.47k stars 490 forks source link

Adding audiobook script #1407

Closed codeperfectplus closed 1 year ago

codeperfectplus commented 1 year ago

Description

Added a script to Read Book in audiobook folder.

Fixes #1406

Type of change

Checklist:

ghost commented 1 year ago
👇 Click on the image for a new way to code review - Make big changes easier — review code in small groups of related files - Know where to start — see the whole change at a glance - Take a code tour — explore the change with an interactive tour - Make comments and review — all fully sync’ed with github [Try it now!](https://app.codesee.io/r/reviews?pr=1407&src=https%3A%2F%2Fgithub.com%2FHarshCasper%2FRotten-Scripts)

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map Legend

codeperfectplus commented 1 year ago

Please merge it after 31 October.

ClasherKasten commented 1 year ago

Please merge it after 31 October.

That's possible, but can I ask what the reason is for this request? (Sorry if I am a little too curious).

codeperfectplus commented 1 year ago

Overall, this is just like an example and documentation for the audiobook library. This is a repository for useful scripts and not a place to promote self-created libraries. My point here is mainly the README.md. It's just the same as audiobooks README.md. But it shouldn't be that. The README.md for this script should be about the the script that you want to add and not about the library behind it. You can also read about this in the STYLE_GUIDELINES.md under section "Prerequisites". If you need inspiration for the README.md take a look at TEMPLATE_README.md file.

script.py itself looks OK to me.

Thanks for pointing out the issues. I will make the necessary changes.

codeperfectplus commented 1 year ago

Please merge it after 31 October.

That's possible, but can I ask what the reason is for this request? (Sorry if I am a little too curious).

I am participating in Hacksquad so I can only make quality PR. Otherwise, I can be disqualified. That's why I was asking to merge it after 31st October. 😁

ClasherKasten commented 1 year ago

As I reviewed this PR the first time I thought that this would be a very cool addition to the project. Problem:

I am participating in Hacksquad so I can only make quality PR. Otherwise, I can be disqualified. That's why I was asking to merge it after 31st October. :grin:

You state yourself that this is not a quality PR. And even if I think this script would be worth adding, I don't think this project aims to be a collections basin for unqualitative PRs and scripts. Is there anyone from the other maintainers (@HarshCasper, @iamakkkhil, @sohamsshah, @seema1711, @vybhav72954, @RohiniRG) who has an (other) opinion on this?

codeperfectplus commented 1 year ago

but it's a very helpful script to convert the book to audio format. so, In my opinion, it's worth it. I mentioned the quality PR which means contributions to the collections repo and DSA repo are not allowed

ClasherKasten commented 1 year ago

I mentioned the quality PR which means contributions to the collections repo and DSA repo are not allowed

Following this argument, you already should be disqualified for opening PRs in awesomeScripts as this is basically the same as this project. So I personally don't see any reason for not merging it before 31. October.

codeperfectplus commented 1 year ago

I mentioned the quality PR which means contributions to the collections repo and DSA repo are not allowed

Following this argument, you already should be disqualified for opening PRs in awesomeScripts as this is basically the same as this project. So I personally don't see any reason for not merging it before 31. October.

I got warnings for that. No issue you can merge it.

ClasherKasten commented 1 year ago

I got warnings for that. No issue you can merge it.

I didn't no this. Now the situation is understandable, I guess this doesn't gonna get merged before 31st October.

codeperfectplus commented 1 year ago

I got warnings for that. No issue you can merge it.

I didn't no this. Now the situation is understandable, I guess this doesn't gonna get merged before 31st October.

Thanks @ClasherKasten