refactor: idiomatic cleanup of EvisionPrecompiled deploy path - #330
Merged
Conversation
Tighten the precompiled download/verify/unarchive/deploy flow:
- verify_checksum/3 now returns a consistent `:ok | {:error, reason}` and owns
its diagnostic message, instead of a 3-tuple on success and a 2-tuple on
error. The old shape raised a MatchError whenever a cached tarball's basename
was absent from the checksum map, because both callers destructured a 3-tuple.
- cached_tarball_usable?/1 and prepare/8 pattern-match that result via `case`.
- nif_installed?/1 replaces the identical .so/.dll path and existence checks
that were duplicated between prepare/8 and run/1.
- deploy_from_dir!/2 drops redundant `File.exists?` guards (File.rm_rf! is a
no-op on missing paths) and uses File.cp_r! instead of a with/else/raise.
- unarchive!/2 and run/1 replace single-clause `with/else` with `case`.
No change to the happy path; the checksum-map-miss case now surfaces the
"run mix evision.fetch" hint instead of crashing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #329. Idiomatic cleanup of the precompiled download → verify → unarchive → deploy flow in
Mix.Tasks.Compile.EvisionPrecompiled. Net -74 lines, no functional change to the happy path.Correctness
verify_checksum/3returned a 3-tuple{matched?, algo, hash}on success but a 2-tuple{:error, reason}when the file was absent from the checksum map. Both call sites destructured a 3-tuple, so a cached tarball whose basename wasn't listed raised aMatchErrorinstead of surfacing the "runmix evision.fetch" hint. It now returns a consistent:ok | {:error, reason}and owns its diagnostic message; callerscaseon it.Idiomatic cleanups
nif_installed?/1collapses the identical.so/.dllpath + existence check that was duplicated verbatim betweenprepare/8andrun/1.deploy_from_dir!/2drops redundantif File.exists?guards (File.rm_rf!is already a no-op on missing paths) and usesFile.cp_r!in place of awith/else/raiseblock.unarchive!/2andrun/1replace single-clausewith/elsewithcase(per Elixir's anti-patterns guide).prepare/8derives the tarball name asname <> ".tar.gz"instead of a second 8-argfilename/8call, and the download block reads as a linear sequence.Scope
Intentionally left the long positional parameter lists (
filename/get_download_url/prepare/run) as-is. Modeling them as a build-spec struct would be gold-plating for a build script, so it was skipped rather than abstracted for its own sake.Verification
mix format.mix.exsand uses the new interfaces; the siblingmix evision.fetchtask does not touch any of them.