Skip to content

refactor: use System.stop/1 to enable caller to rescue tasks #35

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

Merged
merged 2 commits into from
Jun 9, 2025

Conversation

half-shell
Copy link
Contributor

📖 Description and reason

Hello 👋 ,
First of all, thank you for mix_audit and your work.

I am currently working on a small package that just does a bunch of different checks on elixir repositories. One of these check is a scan of the repository's dependencies, and I am using mix_audit for that.

So I have a Mix task calling another one: mix deps.audit; it's just a tiny wrapper over it.

However, the way I am calling it, through a small .exs script, makes it so I can't really call it using System.cmd/3 (for remote reasons, but basically, it doesn't find the task that way because of the way I install mix_audit and the execution context of the script). Considering that you are using System.halt/1, it's the only way for me to prevent the process calling the deps.audit task from shutting down the other processes from my "meta" task.

A simple change that makes my life incredibly easier is using System.stop/1 instead. That let's me call the deps.audit task through Mix.Task.run/2, and handle it that way.

I don't see any downside to this change as it does not change the current behaviour, but you might have had a good reason to use System.halt/1 in the first place.

If that's the case, I'd love to hear about it.

👷 Work done

Tasks

  • Replace all System.halt/1 calls to System.stop/1 calls

🎉 Result

This change has no functional impact whatsoever

🦀 Dispatch

#dispatch/elixir

@mirego-builds mirego-builds requested a review from lewazo May 6, 2025 16:08
@mirego-builds
Copy link

🦀 Requesting reviewers for this pull request:

  • @lewazo (reviewer for the elixir stack)

🦀 Mentionning users for this pull request:

@half-shell
Copy link
Contributor Author

Hi @lewazo & @gmercier42 👋
Would you be able to review this small PRs ? 🙏
It'd help me tremendously
Thanks !

@remi
Copy link
Member

remi commented Jun 9, 2025

@half-shell I’ve updated the branch so the CI is running. I’ll merge it afterwards!

@remi remi merged commit 1469864 into mirego:main Jun 9, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants