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

kroki support? #191

Closed
mskyaxl opened this issue Apr 11, 2021 · 10 comments
Closed

kroki support? #191

mskyaxl opened this issue Apr 11, 2021 · 10 comments

Comments

@mskyaxl
Copy link
Contributor

mskyaxl commented Apr 11, 2021

Hi, have you considered adding asciidoctor-kroki in this container?
https://github.com/Mogztter/asciidoctor-kroki

@mojavelinux
Copy link
Member

I consider Kroki to be a partner community. Therefore, I think it's reasonable for it to be included in the container (assuming it doesn't cause too much weight). Though we are still facing the problem that this container is growing without bounds, so layers are something we really need as the number of projects people want to include increases.

@SilentButeo2
Copy link

I don't know how Kroki matches up with the current implemented Asciidoctor Diagram.
What I can say, is that although Asciidoctor Diagram supports a bunch of extensions, it seems that not all of them (or maybe most of them) are not working in this docker container.

Currently I have a document to convert where multiple diagram processor [svgbob][wavedrom][mermaid][plantuml].
And its only the [plantuml] that currently works.

So I think that Kroki would fix that part. Asciidoctor Diagram could be removed?
Or don't include Kroki, but make Asciidoctor Diagram implementation work for all diagram processors?

@dduportal
Copy link
Contributor

Hello @SilentButeo2 @mskyaxl , would you be interested in helping us by proposing a PR to install kroki in the container (+ add a test in the test harness with a minimalist example)?
If you are interested but need help/mentoring I can be there for you.

@mskyaxl
Copy link
Contributor Author

mskyaxl commented Oct 19, 2021

i have already a branch that does this, I can update it(if needed) and create a PR :)

@dduportal
Copy link
Contributor

@mskyaxl that would be awesome! We'll check the size differences to ensure the new image is not bloated, but that could be great

@mojavelinux
Copy link
Member

I would be supportive of adding Asciidoctor Kroki to the image, but not removing Asciidoctor Diagram. I don't want to create a situation where we put two projects in this ecosystem against each other. Asciidoctor Diagram and Kroki take different approaches to handling the work of diagram generation. The benefit of Asciidoctor Diagram is that it works completely locally. Asciidoctor Kroki offloads the work of generating the diagram to another server (which is why it's easier to support more diagram types without adding software). And since the provider is remote, adding the extension should be very lightweight.

If we get to a point where we offer several different images, I could see having one image that uses Asciidoctor Diagram and all its diagram tool dependencies and another image having Asciidoctor Kroki to keep the image more lean.

@SilentButeo2
Copy link

The reason I only would like to have Asciidoctor Kroki support, is that the current Asciidoctor Diagram doesn't seem to support [svgbob][wavedrom][mermaid] (and maybe others also).

So support for Asciidoctor Kroki is as such not mandatory IMHO.

@mojavelinux
Copy link
Member

mojavelinux commented Oct 20, 2021

Asciidoctor Diagram supports those formats (see https://docs.asciidoctor.org/diagram-extension/latest/#creating-a-diagram), they just may not be fully enabled in this particular image. Each diagramming tool adds bulk to the image, so choices have been made to support only the most popular ones, or ones that people have requested specifically. (See #109 for more context).

@mskyaxl
Copy link
Contributor Author

mskyaxl commented Oct 24, 2021

I created the PR #226.

dduportal added a commit that referenced this issue Nov 7, 2021
[#191] included Asciidoctor Kroki into the docker image
@dduportal
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants