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

flake: Run pre and post hooks in checkPhase #2218

Merged
merged 1 commit into from Mar 21, 2023
Merged

Conversation

afh
Copy link
Member

@afh afh commented Mar 20, 2023

No description provided.

@afh afh added the easy Trivial changes easy to review label Mar 20, 2023
@afh afh requested review from purcell and tbm March 20, 2023 20:09
@afh afh self-assigned this Mar 20, 2023
Copy link
Contributor

@tbm tbm left a comment

Choose a reason for hiding this comment

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

I have no idea what this is doing but I guess you know what you're doing

@tbm tbm merged commit 1db189b into ledger:master Mar 21, 2023
2 checks passed
@afh afh deleted the flake-check-phase branch March 21, 2023 07:11
@afh
Copy link
Member Author

afh commented Mar 21, 2023

Thanks for merging, @tbn 🙏

Nixpkgs builds packages in phases, e.g. configurePhase, buildPhase, installPhase, etc. Each of these phases can be override (ledger's flake overrides the checkPhase) and fine-tuned by defining pre- and postHooks which are executed and the start and end of a phase.

To quote the Nixpkgs documentation about Phases:

When overriding a phase, for example installPhase, it is important to start with runHook preInstall and end it with runHook postInstall, otherwise preInstall and postInstall will not be run. Even if you don’t use them directly, it is good practice to do so anyways for downstream users who would want to add a postInstall by overriding your derivation.

So the changes in this PR ensure that nixpkgs best-practices are followed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy Trivial changes easy to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants