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

Feature: expose basic app options via CLI #48

Merged
merged 1 commit into from
Oct 22, 2022

Conversation

aaaandrzej
Copy link
Contributor

Pull Request Template

script name -

What have you Changed

Added CLI for basic usage of the app.

Issue no.(must) - #42

Self Check(Tick After Making pull Request)

  • This issue was assigned to me.
  • One Change in one Pull Request
  • My file is in proper folder
  • I am following clean code and Documentation and my code is well linted with flake8.
  • I have added README.md and requirements.txt with my script

If issue was not assigned to you Please don't make a PR. It will marked as invalid.

Join Us on Discord:- https://discord.gg/JfbK3bS

@aaaandrzej
Copy link
Contributor Author

Side note to the maintainers - are you sure that https://github.com/apps/auto-assign is installed there? Minor thing but this PR didn't get the reviewers assigned automatically, I believe either the above should be installed or the yaml file should be extended to follow https://github.com/marketplace/actions/auto-assign-issue with on and jobs structure.

@@ -70,7 +66,7 @@ def create_json_book(self, input_book_path, password=None, extraction_engine=Non
)

if os.path.exists(os.path.join(BOOK_DIR, json_filename)):
metadata = {}
metadata = {"book_name": json_filename.split(".")[0]}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing book_name in metadata was triggering an exception when saving audio in case the json file already existed (with no book_name)

Comment on lines +87 to +88
else:
raise NotImplementedError("Only PDF, TXT, EPUB, MOBI, HTTP, DOCX and DOC files are supported")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to add an else condition to avoid errors saying that json_book and metadata are not always specified

Comment on lines +100 to +103
book_dir = os.path.join(BOOK_DIR, book_name)
os.makedirs(book_dir, exist_ok=True)

print("Saving audio files in folder: {}".format(book_name))
print("Saving audio files in folder: {}".format(book_dir))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saving audio to the same directory that contains books makes more sense to me than the actual dir where the code lives

@codeperfectplus
Copy link
Member

so that CLI will not affect the previous code(export as module). we can export it as a module as well.

@codeperfectplus codeperfectplus merged commit 544d26f into Py-Contributors:dev Oct 22, 2022
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