google / gson

A Java serialization/deserialization library to convert Java Objects into JSON and back
Apache License 2.0
23.29k stars 4.28k forks source link

Fix potential github action smells #2684

Open ceddy4395 opened 4 months ago

ceddy4395 commented 4 months ago

Purpose

Hey! 🙂 I've made the following changes to your workflow:

(These changes are part of a research Study at TU Delft looking at GitHub Action Smells. Find out more)

Description

Checklist

google-cla[bot] commented 4 months ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

ceddy4395 commented 4 months ago

@Marcono1234 Thank you for the review! I've made the changes you've suggested, including bumping the timeouts.

I've also made the same fixes to the other workflow files!

eamonnmcmanus commented 4 months ago

Thank you for doing this! Overall it looks good, but I'm not entirely convinced by this:

  • Use fixed version for runs-on argument
    • Using an explicit version for runs-on makes the developers more conscious about on what OS version they are testing/supporting. Furthermore, if breaking changes are introduced in the next version, they will not automatically break the pipeline.

Would Dependabot or something else notify us if the latest supported version changes? Otherwise I'm afraid we'll never think to update it. I think I might rather discover that an Ubuntu update breaks our build as soon as it happens rather than months later, even if that means the CI mysteriously failing.

Marcono1234 commented 4 months ago

Would Dependabot or something else notify us if the latest supported version changes?

No, that does not seem to be supported yet, see https://github.com/dependabot/dependabot-core/issues/7182.

eamonnmcmanus commented 4 months ago

Would Dependabot or something else notify us if the latest supported version changes?

No, that does not seem to be supported yet, see dependabot/dependabot-core#7182.

Thanks for that pointer. In that case, I'd suggest we leave this part as it is (with ubuntu-latest) until Dependabot does have that support.