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

Adapting your github matrix strategy for screenplay writing #115

Open
stevenjaycohen opened this issue Sep 4, 2023 · 7 comments
Open

Adapting your github matrix strategy for screenplay writing #115

stevenjaycohen opened this issue Sep 4, 2023 · 7 comments
Labels

Comments

@stevenjaycohen
Copy link

I found your explanation for using a matrix on a github action to scan directories and I was trying to adapt it for this purpose:

Versioning Scripts using Fountain and GitHub

As you can see, he stops just short of your fix and suggests adding each new file manually.

I have been trying to apply your solve but I keep missing something and generating errors. I would truly appreciate your help, if you have the time.

Admittedly, this is quite a bit simpler than your issue. I just need the names of the files ending in .fountain, strip the .fountain from the name, put the names into the matrix, then adapt the run line so ${{ matrix }}.fountain and ${{ matrix }}.pdf point to the right things.

@dblock
Copy link
Owner

dblock commented Sep 5, 2023

Happy to help. Show me what you have tried and how it's (not) working?

@stevenjaycohen
Copy link
Author

stevenjaycohen commented Sep 5, 2023

The "errors" were not really that helpful (see below) It just refused to run.

image

So, I'm starting over below...

The Original Code (no changes)

name: fountain_to_pdf

 on:
   push:
     branches:
       - main
   workflow_dispatch:

 jobs:
   convert_files:
     name: Convert Fountain to PDF
     runs-on: ubuntu-latest
     permissions:
       contents: write
     steps:
       # Check out your repository
       - uses: actions/checkout@v3
          
       # Install 'afterwriting
       - name: Install 'afterwriting
         run: npm install afterwriting -g

       # Invoke 'afterwriting to export script.fountain to script.pdf
       - name: Generate PDF
         run: afterwriting --source script.fountain --pdf pdf/script.pdf --overwrite --config config.json

       # Commits changes
       - uses: stefanzweifel/git-auto-commit-action@v4

My Try (based upon what I didn't quite understand)

Note: My attempts to leverage your code are preceded by #COMMENTS

name: fountain_to_pdf

 on:
   push:
     branches:
       - main
   workflow_dispatch:

 jobs:

#I NEED THIS NEW JOB HERE?
   find_files:
    runs-on: ubuntu-latest
    outputs:
      matrix: ${{ steps.set-matrix.outputs.matrix }}
    steps:

#SHOULD THIS REMAIN CHECKOUT@V2 OR SHOULD IT BE @V3 LIKE THE ORIGINAL SCRIPT?
      - uses: actions/checkout@v2
      - id: set-matrix

#I NEED TO LS THE BASE DIRECTORY FOR .FOUNTAIN FILES
#I DON'T KNOW HOW TO MODIFY THE JQ COMMAND
#IDEALLY I NEED THE FILE NAMES WITHOUT THE .FOUNTAIN EXTENSION 
        run: echo "::set-output name=matrix::$(ls *.fountain | jq -R -s -c 'split("\n")[:-1]')"

   convert_files:

#IS THIS WHERE THE NEEDS ITEM GOES?
needs: find_files

     name: Convert Fountain to PDF
     runs-on: ubuntu-latest

#DO I NEED STRATEGY?
strategy:

#HAVE I MODIFIED THE MATRIX PROPERLY?
    matrix: ${{ fromJson(needs.find_files.outputs.matrix) }}

     permissions:
       contents: write
     steps:
       # Check out your repository
       - uses: actions/checkout@v3
          
       # Install 'afterwriting
       - name: Install 'afterwriting
         run: npm install afterwriting -g

       # Invoke 'afterwriting to export script.fountain to script.pdf
       - name: Generate PDF

#SINCE I DON'T NEED THE MANIFEST CODE, IS THIS CORRECT?
#DOES CALLING THE MATRIX TWICE LIKE THIS GET THE SAME VALUE (which i need) OR SEQUENTIAL VALUES?
         run: afterwriting --source ${{ matrix }}.fountain --pdf pdf/${{ matrix }}.pdf --overwrite --config config.json

       # Commits changes
       - uses: stefanzweifel/git-auto-commit-action@v4

Thank you for taking the time to look this over. I really appreciate it.

@dblock
Copy link
Owner

dblock commented Sep 5, 2023

Do you have a repo where you're trying this? There should be a detailed log with your job linked under those ....

I would start adding the new parts one by one before trying to whole thing. For example, just leave find_files in there and see if you can get that one to work?

@stevenjaycohen
Copy link
Author

I almost have it! I'm just not sure how to strip the filetype .fountain from the matrix field (see below). Any ideas??

image

Also, you may want to update your checkout@v2 to @V3. When I was going through my errors, that resolved a warning that originated in the code that I copied from your project.

@dblock
Copy link
Owner

dblock commented Sep 6, 2023

Hard to tell, you'll have to show your GHA source.

@dblock
Copy link
Owner

dblock commented Sep 6, 2023

Also, you may want to update your checkout@v2 to @V3.

Appreciate a PR if you know where this is.

@stevenjaycohen
Copy link
Author

So, I am running into an odd thing that is likely out of scope here. But, to let you know, it appears that even if I git pull/push/etc to be completely up to date, the system seems to get itself into a state where it thinks that SOME of the PDFs can/cannot be updated. So, I wind up with random failures in each run.

Not a fault related to what we are doing, likely an issue in how the fountain converter works.

image

find_files is me figuring out how to leverage your code to generate the matrix properly thank you!!

find_files
The set-output command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

find_files
The following actions uses node12 which is deprecated and will be forced to run on node16: actions/checkout@v2. For more info: https://github.blog/changelog/2023-06-13-github-actions-all-actions-will-run-on-node16-instead-of-node12-by-default/

I solved the second of those alerts by changing checkout@v2 to checkout@v3 without causing another issue. I did not resolve that first one.

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

No branches or pull requests

2 participants