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

Is there any possibility to use file as a script? #38

Closed
extempl opened this issue Apr 1, 2020 · 13 comments
Closed

Is there any possibility to use file as a script? #38

extempl opened this issue Apr 1, 2020 · 13 comments

Comments

@extempl
Copy link

extempl commented Apr 1, 2020

Maybe using cat file or something?
If no, is it possible to add?

Because it is not that easy to write normal js code inside yaml.

@extempl
Copy link
Author

extempl commented Apr 17, 2020

End up with following solution:

      - run: echo "::set-output name=js::$(tr '\n' ' ' < ./.github/diffWithMaster.js)"
        id: diffJS
        name: service run

      - name: Getting diff with master
        id: diff
        uses: actions/github-script@0.9.0
        with:
          any_variable: ${{github.event.ref || github.event.pull_request.head.ref}} 
          github-token: ${{secrets.GITHUB_TOKEN}}
          result-encoding: string
          script: ${{ steps.diffJS.outputs.js }}

tr '\n' ' ' for merging file into one line, so make sure you have ; in the js everywhere and don't use // comments, use block one instead.
Also, so far this is the only solution to nicely merge output (echo won't work on github actions for some reason).

Also note that in this case need to throw external variables inside script through with, and read in the script with core.getInput('any_variable').

Js script may be safely wrapped into async function to get correct syntax and avoid errors:

/* global github, context, core */
(async () => {
/*....code (remember about semicolons) */
})();

But in this case you won't be able to retrieve any returns from the script. This probably could be solved on the github-script side.

@DannyvanderJagt
Copy link

Using files instead of strings would be nice. Will save a lot of code in my .yml files and allows me to re-use scripts I already use for other stuff.

I made a local PR to add file support. I'm testing it now and will create a PR here tomorrow.

@lukka
Copy link

lukka commented May 15, 2020

Having arbitrary code to run inside to the yaml file, it is not a good idea, for many reasons:

  • not testable with unit test;
  • not manually testable;
  • maintenance headache;

IMO yaml is good to contain declarative statements only and it should be as concise as possible.

Having file as an input should be coming from the design review, not from customer feedback.

@extempl
Copy link
Author

extempl commented May 15, 2020

Having file as an input should be coming from the design review, not from customer feedback.

In fairness, it is true when you selling your product, and there are customers. Otherwise, when you building some tool for yourself and publishing to help others - it is not really fair to say like that :)

@lukka
Copy link

lukka commented May 15, 2020

My fault, I completely missed this action is experimental indeed as stated in the README.md file. I take it back :)

lukka added a commit to lukka/github-script that referenced this issue May 15, 2020
@jclem
Copy link
Contributor

jclem commented May 18, 2020

@lukka We're not going to remove the ability to inline the script—we've seen that there's a huge demand for it in development lifecycle flows where a user just wants to write one file that accomplishes the task of something like commenting on an issue. Also, I use it myself a huge amount in quick one-off workflows that I don't want to put much effort into 😆

That said, I also think there's value in being able to run a file, and I plan on adding this soon.

@jclem
Copy link
Contributor

jclem commented May 18, 2020

The more I think about it, the less I'm convinced that adding this is necessary. If you want to run a file instead of a plain script, it can be done this way:

uses: actions/github-script@v0.9.0
  with:
    script: |
      const path = require('path')
      return require(path.resolve('my-script.js'))({context})

And then, in your file:

module.exports = ({context}) => {
  return context.payload.client_payload.value
}

It's also safe to use a module exporting an async function in this way as long as you await it in the inline script.

@extempl
Copy link
Author

extempl commented May 18, 2020

@jclem I think any solution would work, as long as it is documented. So, yours actually looks great, but I'd add it to the readme.

@jclem
Copy link
Contributor

jclem commented May 18, 2020

@extempl Thanks—does this seem clear? https://github.com/actions/github-script#run-a-separate-file

@jclem
Copy link
Contributor

jclem commented May 18, 2020

In the meantime, I'm going to go ahead and close this issue.

While it's not quite as simple on the user-side as adding a file as an input to the Action, it has the benefit of:

  • Letting the external file be a testable module
  • Keeping this action focused on its goal of enabling quick one-off actions contained entirely in the workflow file itself

@jclem jclem closed this as completed May 18, 2020
@extempl
Copy link
Author

extempl commented May 18, 2020

Yes, the only thing is missing I think, is the note about mandatory checkout

@jclem
Copy link
Contributor

jclem commented May 18, 2020

the note about mandatory checkout

Good call, I will add that.

@sr-apotapov
Copy link

sr-apotapov commented Apr 5, 2021

While it's not quite as simple on the user-side as adding a file as an input to the Action, it has the benefit of:

  • Letting the external file be a testable module
  • Keeping this action focused on its goal of enabling quick one-off actions contained entirely in the workflow file itself

@jclem Can you please explain how exactly can people use it?
For example I'm trying to use {{secret.NAME}} and {{steps.NAME.outputs.stdout}} context. What should I pass exactly? in yaml and how should I change my JS code?

v1v added a commit to elastic/apm-pipeline-library that referenced this issue Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants