Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(manager/asdf): add additional asdf supported tools #18612

Merged
merged 18 commits into from Nov 15, 2022
Merged

feat(manager/asdf): add additional asdf supported tools #18612

merged 18 commits into from Nov 15, 2022

Conversation

maennchen
Copy link
Contributor

@maennchen maennchen commented Oct 27, 2022

Changes

Adds support for the following tools to the asdf manager:
awscli, bun, cargo-make, clojure, crystal, deno, direnv, dprint, elixir, elm, erlang, gauche, golang, haskell, helmfile, helm, hugo, idris, java, julia, just, kotlin, kustomize, lua, nim, nodejs, ocaml, perl, php, python, ruby, rust, scala, shellcheck, shfmt, terraform, trivy, zig

Context

Currently, only nodejs is supported.

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

Test Repo: https://github.com/jshmrtn/renovate-asdf-test

Code is largely extracted from:
https://github.com/kachick/renovate-config-asdf/tree/1.8.0/plugins

Thanks @kachick!

Adds support for:
awscli, bun, cargo-make, clojure, crystal, deno, direnv,
dprint, elixir, elm, erlang, gauche, golang, haskell, helmfile,
helm, hugo, idris, java, julia, just, kotlin, kustomize, lua, nim,
nodejs, ocaml, perl, php, python, ruby, rust, scala, shellcheck,
shfmt, terraform, trivy, zig

Code is largely extracted from:
https://github.com/kachick/renovate-config-asdf/tree/1.8.0/plugins

Thanks @kachick!
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

maennchen and others added 2 commits October 28, 2022 16:00
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
Copy link
Contributor

@multani multani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least these tools are fetched from GitHub releases: using the GitHub tag can create a race condition where the tag has been published but not the release yet, which will fail asdf it the update is merged.

I implemented these on @kachick repository and that was an oversight from me, if we can have the fixed version here, that would be great!

lib/modules/manager/asdf/upgradeable-tooling.ts Outdated Show resolved Hide resolved
lib/modules/manager/asdf/upgradeable-tooling.ts Outdated Show resolved Hide resolved
lib/modules/manager/asdf/upgradeable-tooling.ts Outdated Show resolved Hide resolved
lib/modules/manager/asdf/upgradeable-tooling.ts Outdated Show resolved Hide resolved
@viceice
Copy link
Member

viceice commented Oct 31, 2022

At least these tools are fetched from GitHub releases: using the GitHub tag can create a race condition where the tag has been published but not the release yet, which will fail asdf it the update is merged.

I implemented these on @kachick repository and that was an oversight from me, if we can have the fixed version here, that would be great!

sounds good. we should use releases when they are used by those tool instead of git tags

@multani
Copy link
Contributor

multani commented Oct 31, 2022

At least these tools are fetched from GitHub releases: using the GitHub tag can create a race condition where the tag has been published but not the release yet, which will fail asdf it the update is merged.
I implemented these on @kachick repository and that was an oversight from me, if we can have the fixed version here, that would be great!

sounds good. we should use releases when they are used by those tool instead of git tags

Yes, I guess it could be applied to other tools, but it's probably plugin dependent, so in theory, it should be configured on a case by case basis ...

maennchen and others added 2 commits November 2, 2022 13:28
Co-authored-by: Jonathan Ballet <jon@multani.info>
@maennchen maennchen requested review from multani and removed request for viceice November 2, 2022 12:32
multani
multani previously approved these changes Nov 3, 2022
Copy link
Contributor

@multani multani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a look at all the links posted and make further adjustments.

Otherwise, this looks great!

Worst case scenario:

  • The config references a git tag instead of a release: there may be a slight race condition and the new version can't be downloaded, but I guess it really happens for a short time.
  • The config references a GitHub Release but only git tags are published: no updates will be proposed and Renovate's own config will have to be fixed. No harms done, bugs happen :)

Also, this is really tied up to the current implementation of each plugin and how upstream packages are released, so 🤷

Thanks a lot for importing all of this!

lib/modules/manager/asdf/upgradeable-tooling.ts Outdated Show resolved Hide resolved
lib/modules/manager/asdf/readme.md Outdated Show resolved Hide resolved
lib/modules/manager/asdf/readme.md Outdated Show resolved Hide resolved
Co-authored-by: Jonathan Ballet <jon@multani.info>
@HonkingGoose HonkingGoose mentioned this pull request Nov 7, 2022
6 tasks
@maennchen maennchen requested review from viceice and rarkins and removed request for viceice November 7, 2022 11:53
rarkins
rarkins previously approved these changes Nov 8, 2022
@rarkins rarkins requested a review from viceice November 8, 2022 16:08
@maennchen maennchen dismissed stale reviews from multani and rarkins via 60924c6 November 9, 2022 12:23
@maennchen
Copy link
Contributor Author

@viceice I added the tests 👍

lib/modules/manager/asdf/index.spec.ts Outdated Show resolved Hide resolved
lib/modules/manager/asdf/index.spec.ts Outdated Show resolved Hide resolved
@maennchen maennchen requested review from viceice and removed request for kachick November 10, 2022 13:08
@viceice viceice enabled auto-merge (squash) November 14, 2022 21:54
@viceice
Copy link
Member

viceice commented Nov 14, 2022

next time you should open the PR from personal repo, otherwise we can't update your branch, with is needed for merge

@maennchen
Copy link
Contributor Author

@viceice Would you prefer for me to open a new PR from my personal account to replace this one?

@viceice
Copy link
Member

viceice commented Nov 15, 2022

no, just use the github update button, and hope we get this merged fast enough before next main branch commit

@viceice viceice merged commit 3d68c7e into renovatebot:main Nov 15, 2022
@maennchen maennchen deleted the extend-asdf-support branch November 15, 2022 10:15
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 34.25.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants