gha-common / go-beautiful-html-coverage

A GitHub Action to track code coverage in your pull requests, with a beautiful HTML preview, for free.
https://gha-common.github.io/go-beautiful-html-coverage/go-test-app-01/head/head.html#file1
MIT License
4 stars 0 forks source link

fix: install vim if not present #56

Open kilianc opened 1 month ago

kilianc commented 1 month ago

this is helpful when gha run in containers setups running in containers

ccoVeille commented 1 month ago

Van you explain the use case?

My understanding of the project is still limited, but I don't get when vim could be needed.

Having context would explain to understand why to set it up, and so to understand what to set up, and sometimes how

kilianc commented 4 weeks ago

See here: https://github.com/gha-common/go-beautiful-html-coverage/blob/main/scripts/beautify-html.sh#L18-L23

Otheriwise, like here people need to install vim if not present.

This represents a couple of challenges:

  1. we don't know what package manager they use
  2. we don't know what os they use

I think supporting the most popular case on github is enough and people can contribute more if needed.

ccoVeille commented 4 weeks ago

Thanks ks for your explanation it's now clearer.

I'm still doubtful about the need to install vim to use only ex

I commented the PR you linked accordingly

kilianc commented 3 weeks ago

the reason why sed is not the best in this case is beacause replace in place is not POSIX. https://stackoverflow.com/questions/5694228/sed-in-place-flag-that-works-both-on-mac-bsd-and-linux

ccoVeille commented 3 weeks ago

I'm still unsure to understand.

The problem seems to be related to sed with macOS, but then gsed (available via gnu-sed) would help but then here is my problem.

Is there anyone using your action on a macOS image 🤔 I mean the code you pointed out was using ubuntu-latest so an image were sed -i does in place replace

ccoVeille commented 3 weeks ago

Oh you said, the GitHub action doesn't know which package manager is available, and the OS

But what about adding a shell script to launch sed this way

tmp=$(mktemp)
sed 'whatever' file > $tmp
mv $tmp file

Then no vim needed

kilianc commented 3 weeks ago

I am ok with an alternative solution

ccoVeille commented 3 weeks ago

@shoenig did this

https://github.com/shoenig/test/pull/157/files

and it's very simple