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

add support for ebooks #63

Merged
merged 14 commits into from
Sep 10, 2024
Merged

add support for ebooks #63

merged 14 commits into from
Sep 10, 2024

Conversation

danvergara
Copy link
Owner

@danvergara danvergara commented Aug 28, 2024

Add support for ebooks

Description

This PR adds support for some of the most common e-books. To do this I had to embed the ebook-convert binary from the Calibre project. The tool is installed on the Container image.

Fixes #60

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Added more tests and QA'd the new conversions.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have checked my code and corrected any misspellings

@danvergara danvergara changed the title add support to convert pdf files to epub add support for ebooks Aug 28, 2024

Choose a reason for hiding this comment

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

Veo muy largo el archivo, quizá valga la pena separarse por responsabilidades o funciones.

if err != nil {
return nil, fmt.Errorf("error reading zip file: %v", err)
}

return bytes.NewReader(zipFile), nil
case EPUB:

Choose a reason for hiding this comment

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

El case para EPUB es muy largo, ¿por qué no lo metes en un método aparte el cuerpo de ese case?

Choose a reason for hiding this comment

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

Coincido con Sirpyerre aqui. Lo haria en un metodo aparte.

if err != nil {
return nil, fmt.Errorf("error reading zip file: %v", err)
}

return bytes.NewReader(zipFile), nil
case EPUB:

Choose a reason for hiding this comment

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

Coincido con Sirpyerre aqui. Lo haria en un metodo aparte.

@danvergara
Copy link
Owner Author

@ThiagoMowszet @Sirpyerre I'm not done with this one, guys. I appreciate the early review, but I'm far from done adding support for ebooks. When the time to properly review this, I'll move the PR from draft mode to ready for review.

A refactor is dramatically needed but I'm more focused on move things along. When I reach a desired stated, I'll go over the code and build a solution off the most common patterns.

I'll let you know, guys.

@danvergara danvergara marked this pull request as ready for review September 6, 2024 04:13
@danvergara danvergara self-assigned this Sep 6, 2024
@danvergara danvergara added enhancement New feature or request file:document Feature or bugfix that has to do with documents labels Sep 6, 2024
@danvergara danvergara merged commit 1d52b49 into main Sep 10, 2024
1 check passed
@danvergara danvergara deleted the add-ebook branch September 10, 2024 04:34
@ThiagoMowszet
Copy link

@ThiagoMowszet @Sirpyerre I'm not done with this one, guys. I appreciate the early review, but I'm far from done adding support for ebooks. When the time to properly review this, I'll move the PR from draft mode to ready for review.

A refactor is dramatically needed but I'm more focused on move things along. When I reach a desired stated, I'll go over the code and build a solution off the most common patterns.

I'll let you know, guys.

Thanks for the update @danvergara!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request file:document Feature or bugfix that has to do with documents
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature request] book format conversion
3 participants