Various code cleanups and modernisations

[Jacalz]

Just a small tidy to keep code quality up and in sync with Fyne v2.5.0.

[andydotxyz]

Thanks for this

[andydotxyz]

It looks like the CI is not matching the versions required in this update, so I guess it also needs to be changed to match what Fyne is using upstream?

[Jacalz]

Good point. Should be fixed now

[Jacalz]

Ping. Would you mind doing a re-review of this? :)

[andydotxyz]

Sorry this dropped off my radar - looks good thanks

[Jacalz]

I had forgot that the Go update made changes to comment formatting. I have now re-formatted the code 👍

[andydotxyz]

Any idea what the failure means @Jacalz ?

[Jacalz]

It was missing some package. Sorry about that. Should be working now 😅

[andydotxyz]

Thanks

[andydotxyz]

Hmm, the changes don't seem to have fixed the static-check failure. I don't understand the message, do you?

[Jacalz]

It is a very strange error. From what I can tell, the test fails when testing with coverage data enabled because it tries to compare the binaries and they no longer match (due to different coverage?). I added a change to skip the tests when we get to that point. We can probably update it in the future to not run it on the ci tag but I didn't have time to implement that right now.

[andydotxyz]

Can this skips really have dropped this 20% in coverage?

[Jacalz]

Seems very strange. I tried to rectify it by not calling t.Skip() but instead returning thinking that a skip made the whole test not count towards the coverage. I have no idea if it works or not but seems not to change anything from my local tests. I only have Go 1.22 available so I can't compare before and after outputs with -cover given that those fail locally for me as well.

[andydotxyz]

Hmm, no change with this new approach @Jacalz.

Oh I see what has happened - it added the cmd folder compare to previous test coverage.
This was not caused directly by the code changes. Do you want to revert the new skip approach as the last one was less code?

[andydotxyz]

Commented on the thread - seems the coverage issue was not that test after all, do you want to go back to the previous approach, or leave as is?