Dfoulks/asciidoc by dfoulks1 · Pull Request #77 · apache/infrastructure-actions

dfoulks1 · 2024-12-03T17:09:27Z

Requesting to add the AsciiDoc action (builds Asciidoc files and commits to perscribed branch)

Uses

checkout@v3

Process

Checks out code
Installs nodejs + asciidoc packages
iterates through adoc files in $srcfiles
moves HTML and $assets to tempdir.
checks out new branch
cleans out cruft
commits new stuff to $dest branch

… tempdir

…the future but this works for now

assignUser

Some comments, will have another look later

asciidoc/action.yml

assignUser · 2024-12-03T17:32:04Z

asciidoc/action.yml

+        git status
+        if git commit -m "Commit build products"
+          then
+              git push


I'm pretty sure this will fail as you are not setting up auth via actions/checkout or something.

this was pulled from the pelican action and has been tested against the infrastructure-presentations repo

I assume the workflow using the pelican action called actions/checkout then.

You could make it a usage requirement to use this within an existing checkout but that should be documented specifically requiring that persist-credentials: is set to true otherwise this will fail.

I think calling the input publish instead of pushToBranch is unfortunate.

assignUser · 2024-12-03T17:35:15Z

asciidoc/action.yml

+runs:
+  using: "composite"
+  steps:
+    - uses: actions/setup-node@v4


Suggested change

- uses: actions/setup-node@v4

- uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0

While it's an offical action and this doesn't require pinning in workflows I think it still makes sense within an action as the user has no control over it?

would there be a way to test this / keep this hash up to date with dependabot or something? Trying to prevent having to touch this everytime there's an update.

yes dependabot can do it, just add the dir with action.yml to the directories list in dependabot.yml

I would pin all actions also the ones coming from GitHub. The positive side effect is that your action does not have uncertainty wrt which exact action is being used. We have seen in the past that sometimes bugs are introduces in newly released versions and when you use just @v4 you will always get the latest released version of the v4 branch of that action. Having it pinned to an exact version makes actions more stable imho.

btw. you can use https://github.com/eclipse-csi/octopin for pinning.

install with pipx install octocpin

assignUser · 2024-12-03T17:37:20Z

asciidoc/action.yml

+          if [[ "$(echo $file | awk -F. '{print $NF}')" != "adoc" ]]; then
+            continue
+          fi
+          echo $file


Does this not make the output very noisy? Maybe use groups to structure the log see https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/workflow-commands-for-github-actions#grouping-log-lines

This was all ripped from the pelican action. If there's a better / more GHA way to approach this, I'm 100% interested.

assignUser · 2024-12-03T17:40:02Z

asciidoc/action.yml

+          fi
+          echo $file
+          node convert-slides.js $file
+          mv $(basename $file .adoc).html "${{ inputs.tempdir }}"/ 


Suggested change

mv $(basename $file .adoc).html "${{ inputs.tempdir }}"/

mv $(basename $file).html "${{ inputs.tempdir }}"/

Or did that serve a purpose there? I think it would just be dropped?

the basename command requires a suffix to strip.

ah that makes sense, if never needed that before :D

janhoy · 2024-12-04T09:12:50Z

The name of this action is not precise. Asciidoc is a markdown format and can be used for writing books, READMEs or as in this case presentation slides with the reveal framework. So a better name for the action would probably be "presentations" or "slides" or "revealjs" or similar.

assignUser

Please also create a workflow to test the action with some sample input and some different options (obv publish is hard to test, we could create a test branch?).

You can also add npm to dependabot with asciidoc as a directory:

  - package-ecosystem: "npm"
    directory: "/asciidoc/"
    schedule:
      interval: "monthly"

assignUser · 2024-12-03T23:24:48Z

asciidoc/action.yml

+        cp "${{ github.action_path }}/package.json" "${{ inputs.tempdir }}"
+        [ -d "${{ github.workspace }}/${{ inputs.assets }}" ] && cp -r "${{ github.workspace }}/${{ inputs.assets }}" "${{ inputs.tempdir }}" || echo "No ${{ inputs.assets }} found!"
+      working-directory: "${{ inputs.srcdir }}"
+


I would add an actions/upload-artifact step here if: ${{ inputs.publish != 'true' }} (or rather if: inputs.publish if you make it a boolean. I'll add a comment above).

That way you could check the created artifacts when running the action in a PR or something. (and we can use it in testing)

you can use to if: toJSON(inputs.publish) or something like that

asciidoc/action.yml

dfoulks1 · 2024-12-05T15:41:03Z

@janhoy: Thanks for the suggestion! I'll update it to asciidoc-slides

Co-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de> Signed-off-by: dfoulks1 <48735727+dfoulks1@users.noreply.github.com>

dfoulks1

responding to comments

dfoulks1 · 2024-12-05T15:45:22Z

asciidoc/action.yml

+          if [[ "$(echo $file | awk -F. '{print $NF}')" != "adoc" ]]; then
+            continue
+          fi
+          echo $file


This was all ripped from the pelican action. If there's a better / more GHA way to approach this, I'm 100% interested.

dfoulks1 · 2024-12-05T15:48:09Z

asciidoc/action.yml

+          fi
+          echo $file
+          node convert-slides.js $file
+          mv $(basename $file .adoc).html "${{ inputs.tempdir }}"/ 


the basename command requires a suffix to strip.

dfoulks1 · 2024-12-05T15:50:45Z

asciidoc/action.yml

+        git status
+        if git commit -m "Commit build products"
+          then
+              git push


this was pulled from the pelican action and has been tested against the infrastructure-presentations repo

…ctions into dfoulks/asciidoc

jsoref · 2025-02-05T18:05:19Z

asciidoc/action.yml

+        )
+      working-directory: "${{ github.action_path }}"
+
+    - name: 


Please name your steps

jsoref · 2025-02-05T18:06:02Z

asciidoc/action.yml

+    - name: 
+      shell: bash
+      run: |
+        mkdir "${{ inputs.tempdir }}"


Suggested change

mkdir "${{ inputs.tempdir }}"

: 'Some name for this step -- the same as the `name:` field -- so that users see this instead of just the first rather useless line of the step when looking at a workflow'

mkdir "${{ inputs.tempdir }}"

jsoref · 2025-02-05T18:09:15Z

asciidoc/action.yml

+          fi
+          echo $file
+          node convert-slides.js $file
+          mv $(basename $file .adoc).html "${{ inputs.tempdir }}"/ 


Never use ${{ ... }} inside a run: block.

Always use:

env: tempdir: ${{ inputs.tempdir }} run: | ... echo "$tempdir"

jsoref · 2025-02-05T18:10:50Z

asciidoc/action.yml

+        cp "${{ github.action_path }}/package.json" "${{ inputs.tempdir }}"
+        [ -d "${{ github.workspace }}/${{ inputs.assets }}" ] && cp -r "${{ github.workspace }}/${{ inputs.assets }}" "${{ inputs.tempdir }}" || echo "No ${{ inputs.assets }} found!"
+      working-directory: "${{ inputs.srcdir }}"
+


you can use to if: toJSON(inputs.publish) or something like that

jsoref · 2025-02-05T18:11:47Z

asciidoc/action.yml

+        git status
+        if git commit -m "Commit build products"
+          then
+              git push


I think calling the input publish instead of pushToBranch is unfortunate.

… repo

…the output dir around

netomi · 2025-02-10T19:13:28Z

asciidoc/action.yml

+      uses: actions/upload-pages-artifact@v3
+      with:
+        path: '${{ inputs.srcdir }}/${{ inputs.output }}'
+    - name: Deploy to GitHub Pages[


seems like a typo at the end '[' ?

dfoulks1 added 30 commits November 26, 2024 10:03

Working on an asciidoc action

5eee76d

Use node to install and not apt

caa4df7

No package file present yet, install packages by name

088342e

use bash to ls files

e7254b1

look for convert.js in the action path, not in the repo

c3d2f25

Updated the installed package list

2f2bbb3

Updated the installed package list again

d2996d0

convert-slides.js needs to be in the same directory as the adoc?

4b1d544

missing quotes on variables

03a9f1e

Better quotes this time

08ed942

split up the environment set up and the actual node command

e84241f

working on getting paths correct

50227f2

Working with the default directory

745a16d

Working with the default directory directive

2bed37c

directive typo

bd92ead

fixed variable name

bd5644a

added echo and modified the bash capture

4ad7b6d

Tweaked action order

1ec87e9

Testing ordering and working directory

b12b4b2

Generating asciidocs works

3b2aae7

Added some file extension checking

96af62a

Missed bash if ending

f4956dc

Bash syntax is not python syntax

088cf57

Bash syntax is still not python syntax

81127a9

Changed default tempdir

873156e

Switched order to verify tempdir is being created

2898a38

Fixed next step

264fa6f

Updated dest_branch name

54d2010

Set the working directory for all steps

e4a527f

Omit the working directory directive to return to the root of the repo

711c2c6

dfoulks1 added 8 commits November 27, 2024 12:59

Copy the static assets explicitly

bd2cd16

typodix

5a1a8ec

Static into the tempdir

ed671d1

typofix

17108ea

Added a step to locate provided assets directory

66d3e2e

Still working on getting a specified static dir to get moved with the…

1da446c

… tempdir

botched the path, now it goes into tempdir and not outputdir

2f608dd

Cleaned up a bit, would like to divine the name of the assets dir in …

f553880

…the future but this works for now

assignUser reviewed Dec 3, 2024

View reviewed changes

assignUser reviewed Dec 4, 2024

View reviewed changes

Apply suggestions from code review

3989dc2

Co-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de> Signed-off-by: dfoulks1 <48735727+dfoulks1@users.noreply.github.com>

dfoulks1 commented Dec 5, 2024

View reviewed changes

assignUser mentioned this pull request Dec 5, 2024

chore(stash): only run test workflow on relevant changes #78

Merged

dfoulks1 added 6 commits January 6, 2025 11:12

asciidoc example

f52a17e

Streamlining asciidoc action to push straight to ghpages

d45f602

Merge branch 'dfoulks/asciidoc' of github.com:apache/infrastructure-a…

f4d1059

…ctions into dfoulks/asciidoc

Added shell to the run command to install npm in the artifact

15747a1

OF: Indentation typo

1af72cc

Added Working Directory to the npm install in lieu of a relative

c22adb5

jsoref reviewed Feb 10, 2025

View reviewed changes

dfoulks1 added 5 commits February 10, 2025 12:03

action.yml no longer needs the tempdir as it's no longer pushing to a…

2c2fb7b

… repo

Attempting to set the working dir as srcdir/output instead of moving …

cd362ac

…the output dir around

Updated workflow permissions to include pages

d1c09a4

Dialing back perms

92ff2dd

Adding permissions elsewhere

d6cabc6

netomi reviewed Feb 10, 2025

View reviewed changes

dfoulks1 added reviewed This actions has had at least one review new submission New custom action submission labels Sep 16, 2025

	- uses: actions/setup-node@v4
	- uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0

	mv $(basename $file .adoc).html "${{ inputs.tempdir }}"/
	mv $(basename $file).html "${{ inputs.tempdir }}"/

	mkdir "${{ inputs.tempdir }}"
	: 'Some name for this step -- the same as the `name:` field -- so that users see this instead of just the first rather useless line of the step when looking at a workflow'
	mkdir "${{ inputs.tempdir }}"

Conversation

dfoulks1 commented Dec 3, 2024

Uses

Process

Uh oh!

assignUser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janhoy commented Dec 4, 2024

Uh oh!

assignUser left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dfoulks1 commented Dec 5, 2024

Uh oh!

dfoulks1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!