labeler icon indicating copy to clipboard operation
labeler copied to clipboard

Published 20 hours ago • verified publisher icon srvaroa

Switch to javascript action instead of docker action

Open ejhayes opened this issue 3 years ago • 3 comments

Thanks for putting this action together, very useful! This pull request improves the overall speed of this action by introducing the following changes:

  • Automatically building and upload compiled go binary to releases - you can see an example in my fork here: https://github.com/ejhayes/labeler/releases/tag/v0.9.1 (see action.tar.gz)
  • Switch from docker action to javascript action. This eliminates the need to build a docker image that compiles this action then runs it. Speed comparison:
    • Docker action (~44s): https://github.com/srvaroa/labeler/actions/runs/780989998
    • Javascript action (~13s): https://github.com/ejhayes/labeler/actions/runs/1651097410

ejhayes avatar Jan 03 '22 21:01 ejhayes

@srvaroa would be great if this could be merged in, let me know if you need any additional info or changes

ejhayes avatar Jan 03 '22 21:01 ejhayes

Hi and apologies for the very late reply, I totally missed this in my inbox.

I don't have a strong opinion on the dockerfile vs. javascript build. However, I have near-zero knowledge of javascript so I'm afraid I would not be able to maintain the js-based build effectively.

srvaroa avatar Jul 16 '22 14:07 srvaroa

@srvaroa As an alternative, are you open to pre-building the Docker image?

falbrechtskirchinger avatar Aug 04 '22 18:08 falbrechtskirchinger

@srvaroa would this PR solve #53?

bennycode avatar Dec 08 '22 09:12 bennycode

Hi @bennycode, I would definitely be happy to produce a pre-build docker image as part of the release. The part of this that I am more reluctant to merge is the switch to JavaScript as I am not able to support that effectively. I may have time to do the prebuilt docker image over the weekend but do feel free to submit a PR with that change

srvaroa avatar Dec 08 '22 11:12 srvaroa

I have taken bits of this PR to improve the overall build time of the action to the same ballpark as the javascript build (~10s) while keeping the Dockerfile (I use a pre-built binary and download it to build the image). As I noted above my main reluctance to merging the js version is my own ability to maintain it.

Let's see if this change gives reasonable execution times. If it doesn't, I'm open to recovering this one.

Thanks @ejhayes for the PR and ideas on it. Very appreciated.

srvaroa avatar Feb 19 '23 22:02 srvaroa