Also install fonts when installing grub

[Itxaka]

Should fix the second part of kairos-io/kairos#1767

[jimmykarily]

Shouldn't this be outside the for loop block?

[Itxaka]

not really, I just missed adding the name here. We only want to warn the user about it if we cant find font X so it needs to be under the loop.

[jimmykarily]

Then I would rename the variable to fontFound and declare it and initialized it to false inside the for loop. This is to make sure we don't accidentally check against the value from a previous iteration if someone changes the code in the future.

[Itxaka]

I just copied the code from the function above when we do the same for the grub modules. I cant really remember now why its done like this, but pretty sure it had its purpose....which I cant remember now XD

[Itxaka]

still, this needs testing, which I currently cant do due to QA of the 2.4.0, and as this is meant to be for the next patch release, this can wait for now.

[Itxaka]

Trying to remember here, maybe it was done that way because we needed all modules and any of those failing should fail, but in this case the fonts are not really needed, its just nice to haver, so maybe the approach has to be different. No need for this variable anymore, can just warn inside the loop and call it a day....

[jimmykarily]

The whole thing can be extracted to an installGrubFonts function (or some other fitting name).

[Itxaka]

just used once in here, not reusable. I dont think it needs the poor visibility of extracting into a function :)

[jimmykarily]

Extracting code to a function has other benefits (testing separately, making sure it's self-contained and that no variables are touched by this code when they shouldn't, etc). Self-contained tasks should live in self-contained functions but we don't seem to follow this rule anywhere else in the code. This is not the proper place to introduce new rules. I'm fine leaving it as is.

[Itxaka]

test failures have nothing to do with this patch, its the http test that its returning a 403

[codecov]

Codecov Report

Merging #144 (3bb0fc1) into main (417f8ce) will decrease coverage by 0.12%.
Report is 3 commits behind head on main.
The diff coverage is 44.82%.

@@            Coverage Diff             @@
##             main     #144      +/-   ##
==========================================
- Coverage   60.73%   60.62%   -0.12%     
==========================================
  Files          37       37              
  Lines        4452     4480      +28     
==========================================
+ Hits         2704     2716      +12     
- Misses       1521     1535      +14     
- Partials      227      229       +2     

Files Changed	Coverage Δ
pkg/utils/grub.go	66.84% <42.85%> (-4.49%)	⬇️
pkg/http/client.go	85.71% <100.00%> (+0.42%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more