-
-
Notifications
You must be signed in to change notification settings - Fork 1k
fix(nix): improve nix-hash check #5883
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
Conversation
currently, it would give a warning when running goreleaser hc, but wouldn't actually fail the check, as the pipe was being skipped if nix-hash wasn't available. this fixes it, so it checks on run instead. Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Deploying goreleaser with
|
Latest commit: |
494cbba
|
Status: | ✅ Deploy successful! |
Preview URL: | https://6f6dbfa1.goreleaser.pages.dev |
Branch Preview URL: | https://improve-nix-hash-handling.goreleaser.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR ensures that the Nix hash availability check fails at runtime instead of merely warning, by moving the availability check into Run
, updating dependencies, and improving related tests.
- Switches the declared dependency from a hardcoded
"nix-hash"
to thenixHashBin
constant. - Removes
hasher.Available()
fromSkip
and adds an explicit availability check inRun
that returns a skip error. - Updates tests: removes the old skip check, adds
TestRunSkipNoNix
, and definesunavailableHasher
.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
internal/pipe/nix/nix.go | Updated Dependencies , simplified Skip , added availability check in Run |
internal/pipe/nix/nix_test.go | Removed outdated skip test, added TestRunSkipNoNix and unavailableHasher implementation |
Comments suppressed due to low confidence (2)
internal/pipe/nix/nix.go:69
- Consider adding a unit test for
Dependencies
to assert it returns the expectednixHashBin
value, ensuring future changes don’t break this contract.
func (Pipe) Dependencies(_ *context.Context) []string { return []string{nixHashBin} }
internal/pipe/nix/nix_test.go:691
- [nitpick] The test-only type
unavailableHasher
could be renamed totestUnavailableHasher
to clarify its purpose and avoid confusion with production types.
type unavailableHasher struct{}
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5883 +/- ##
==========================================
- Coverage 82.71% 82.68% -0.04%
==========================================
Files 165 165
Lines 16521 16521
==========================================
- Hits 13666 13661 -5
- Misses 2262 2268 +6
+ Partials 593 592 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
currently, it would give a warning when running goreleaser hc, but wouldn't actually fail the check, as the pipe was being skipped if nix-hash wasn't available.
this fixes it, so it checks on run instead.