ggrossetie / asciidoctor-web-pdf

Convert AsciiDoc documents to PDF using web technologies
https://asciidoctor.org
MIT License
450 stars 92 forks source link

resolves #456 add a root-folder in distribution bundles (zip files) #460

Closed wh81752 closed 3 years ago

wh81752 commented 3 years ago

Changes for issue #456 . This PR also contains an alternative solution for #459 (terminating Node.js script).

In order to test my changes to GitHub Actions I create this PR. I guess I should test upfront in my own fork. Is that the way to do it?

wh81752 commented 3 years ago

Ok, after some fiddling with GitHub Actions -- especially how to extract version and share version between workflows -- it starts to looks good. The package contents are now within a versioned root folder, i.e.

$ export version=1.0.0-alpha.13         # whatever the next released version will be
$ wget https://github.com/Mogztter/asciidoctor-web-pdf/releases/download/v${version}/asciidoctor-web-pdf-linux-v${version}.zip
$ unzip asciidoctor-web-pdf-linux-v${version}.zip
$ ls asciidoctor-web-pdf-linux-${version}
assets/     chromium/   css/        examples/   fonts/     asciidoctor-web-pdf*

There is one remaining problem:

  1. The version in the published package name starts with a "v" while the version of the root folder is without that "v"
ggrossetie commented 3 years ago

A few nitpicks but otherwise it looks good 👍🏻 Thanks for taking the time to fiddle with GitHub Actions.

The version in the published package name starts with a "v" while the version of the root folder is without that "v"

I don't have a strong opinion whether we should keep the "v" prefix or remove it but it's important that we stay consistent.

wh81752 commented 3 years ago

Btw, would it not be more consistent to have a smoke test for linux similiar to smoke tests for windows and macos?

ggrossetie commented 3 years ago

Btw, would it not be more consistent to have a smoke test for linux similiar to smoke tests for windows and macos?

Why not, the goal was to save some time but it's negligible.

wh81752 commented 3 years ago

How about merging this PR, or is there a problem left?

ggrossetie commented 3 years ago

No it looks good, I will review one more time, potentially make small changes and squash commits before merging 😉

ggrossetie commented 3 years ago

And merged, thanks @wh81752 🤗