-
Notifications
You must be signed in to change notification settings - Fork 151
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: add barcode
extension
#459
Conversation
81ebd8e
to
aa72d39
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jeromeag , many thanks for this feature. It looks good and legit!
Before approving it, there are a few steps left:
- Could you give us a size (approximatively) difference before and after the change (to evaluate the change impact)?
- Could you update your change to use fixed version to the 3 new Gems (following other examples) so it would be documented by the automation, and I could add the version tracking with
updatecli
?
Hi @dduportal,
Here is the before/after results of command Before:
After:
Done ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job, thanks!
- I'm approving this PR and setting up to auto-merge (merging only if checks are passing but the code looks good!)
- This change only adds 2 Mb to the image: it's fine (image size is close to ~390 Mb).
- Once merged, a new release of the
asciidoctor
Docker image will be done to provide this change (and the gnuplot one) through a versioned release - Subsequent PR will be created (I'll take care of it unless you are interested to do it of course) to add version tracking automation (so further Gem updates on these 3 news will end with an automated bump PR ala Dependanbot)
Add barcode feature as described here:
https://docs.asciidoctor.org/diagram-extension/latest/diagram_types/barcode/