search

Automattic / a8c-ci-toolkit-buildkite-plugin

A caching plugin that can be invoked from your build script.
22 stars 5 forks source link

annotate_test_failures: fix nil crash #76

Closed AliSoftware closed 1 year ago

AliSoftware commented 1 year ago

I encountered a crash on @details being nil in this build so I figured I'd make a quick fix for it.

Twinsen81 commented 1 year ago

thanks for fixing that @AliSoftware! When will this change become effective? I still see the error in the latest builds.

AliSoftware commented 1 year ago

@Twinsen81 I can make a new release, though note that because the we also landed an (unrelated) breaking change in trunk the next version will have to be a major version bump 3.0.0. This means that if you want to adopt that new version and your pipeline were using nvm_install in any CI step, you will have to make sure to follow the migration instructions to migrate to the new way of using nvm as you adopt a8c-ci-toolkit 3.0.0 even if all you wanted was to get that annotate_test_failures fix.

Twinsen81 commented 1 year ago

I didn't find any nvm_install calls in our repo. Does that mean I just need to do this to complete the migration?

plugins:
      - automattic/nvm#0.2.1:
          version: 'v18'
AliSoftware commented 1 year ago

I see that nvm_install is indeed not used in Tumblr-Android repo, and that it seems you already migrated it to use automattic/nvm plugin instead 👍

Which means that the fact that nvm_install command will be removed as part of a8c-ci-toolkit#3.0.0 won't affect Tumblr-Android, as it doesn't use it anymore anyway.

So all you'll have to do once I release a8c-ci-toolkit#3.0.0 is to update all the .buildkite/*.yml files to use a8c-ci-toolkit#3.0.0 everywhere it was referencing a8c-ci-toolkit#2.18.1, and that should be enough to get the fix for annotate_test_failures without breaking anything in the repo.

That being said, it won't hurt to also update the other plugins while we'll be at it, like indeed update our current references to automattic/nvm#0.1.0 in Tumblr-Android to the latest version 0.2.1 of the nvm plugin instead (We already use a .nvmrc file at the root of Tumblr-Android repo, so no need to add that version: 'v18' parameter to the nvm plugin declaration though)

AliSoftware commented 1 year ago

Release 3.0.0 has been created and includes this fix, so you can start referencing it in your repos 👍

Twinsen81 commented 1 year ago

Awesome! Thanks!