go-comic-converter
go-comic-converter copied to clipboard
Expose the internal/epub package
Hi @celogeek.
I want to integrate go-comic-converter with one of my projects.
I've looked at the codebase and figured out that the core epub conversion logic is nicely decoupled from the CLI and configuration logic on a github.com/celogeek/go-comic-converter/internal/epub package.
The problem is that this package lies on the internal package. As stated on the feature release notes, in this case, no packages outside of the root github.com/celogeek/go-comic-converter would be able to import what's within the internal package.
I don't know what's the design decision behind doing that, but it would be good to move the github.com/celogeek/go-comic-converter/internal/epub package to something like github.com/celogeek/go-comic-converter/pkg/epub.
Although I could integrate with it using something like os/exec, by exposing the package, go-comic-converter could also integrate at the code level with other Go packages, which is way better.
A good example of what I'm trying to describe here is github.com/golang-migrate/migrate, where the core package has a CLI but also supports being embedded by other packages.
Are there any concerns about doing that? I can spin up a quick PR with it if you wish, as I'll be forking the repo to implement this change anyway.
Hi, I usually put my code in internal, as I move it a lot, and keep only the exposed part outside. in that case the tools that will handle properly the call to ePub package with all the available options.
I have severals sub package that could be shared by putting it outside the internal. The only issue is when I start doing it, I need to take care to avoid doing breaking changes or force a major update.
I will see if I can share the ePub api properly, without the risk of breaking stuff later. Keeping it clean.
Working on it.
I've start a branch. I have a lot of subpackage, which I change a bit the struct Can you take a look and let me know ? I will keep working to improve the result.
Sure. I'll dedicate some time to take a look at the diff tomorrow. Thanks!
@celogeek I've taken a look at the code, and have a couple of suggestions on the API design of the exposed packages.
Could you create a draft PR? So we can move this discussion there and I can point out my suggestions.
here: https://github.com/celogeek/go-comic-converter/pull/27
I was thinking of creating a real interface for epub, with everything to plug hooks like progress, and everything.
Sure. I create a pr but after a while I close it. I haven't got time to work on it.
Please do. I will check.
Sorry, I deleted my previous message by accident ...
I will do a PR soon, from my understanding, only moving the packages out of the pkg/external do the trick but I could be wrong (everything seems to work fine but I lack tests to validate that)
PR link: #40 As I said it is pretty basic and I just moved the folder pkg out of the internal folder
Well. Internals may move a lot while public api remain stable. Exposing everything this way, will make any changes harder, as we want to keep the interface.
I was thinking of a configurable api that can be used by external client or a command line provided like the one we have.
Like New().WithOption(...) Or New(option1, option2...) Like New(WithTitle('test'), WithProfile('ks'),OnProgress(func(p){...}).Write('dest')
I understand you concerns about making everything public, I will try to have a deeper understanding of how things work in the repo, do you have any idea of what you would like to expose ?
If I had to choose from the options provided maybe New(option1, option2...) is better since we could create a global Options struct composed of multiple options struct parsed using a yaml or json file or with the flags provided which could give the opportunity to have maybe some presets for different scenarios
I export epub and epub options in a public way here an example of usage
package main
import (
"github.com/celogeek/go-comic-converter/v3/pkg/epub"
"github.com/celogeek/go-comic-converter/v3/pkg/epuboptions"
)
func main() {
err := epub.New(epuboptions.EPUBOptions{
Input: "mymanga.cbz",
Output: "mymanga.epub",
Image: epuboptions.Image{
Quality: 90,
Manga: true,
HasCover: true,
View: epuboptions.View{
Width: 1200,
Height: 1920,
AspectRatio: 1.6,
},
GrayScale: true,
Resize: true,
Format: "jpeg",
},
Workers: 8,
}).Write()
if err != nil {
return
}
}
The next step is to change the progress to make it configurable. Like:
- OnProcess(current, total)
- OnWrite()
- Done()