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.

Sign up for GitHub

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

Jump to bottom

ci-plutus-benchmark: use separate nix-shells #3460

Merged
merged 1 commit into from
Jul 1, 2021
Merged

ci-plutus-benchmark: use separate nix-shells #3460

merged 1 commit into from
Jul 1, 2021

Conversation

gilligan
Copy link
Contributor

@gilligan gilligan commented Jun 30, 2021 •
edited
Loading

Run the 'cabal bench' commands in nix-shells to ensure that potentially
different haskell dependencies are being actually used.

Pre-submit checklist:

  • Branch
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Relevant tickets are mentioned in commit messages
    • Formatting, materialized Nix files, PNG optimization, etc. are updated
  • PR
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Pre-merge checklist:

  • Someone approved it
  • Commits have useful messages
  • Review clarifications made it into the code
  • History is moderately tidy; or going to squash-merge

@gilligan
ci-plutus-benchmark: use separate nix-shells
4a5f517 
Run the 'cabal bench' commands in nix-shells to ensure that potentially
different haskell dependencies are being actually used.
@gilligan gilligan requested a review from michaelpj June 30, 2021 14:01
@michaelpj
Copy link
Contributor

It's already in a nix-shell right? So this is going to nest them? Is that... sensible?

@gilligan
Copy link
Contributor Author

gilligan commented Jun 30, 2021 •
edited
Loading

It's already in a nix-shell right? So this is going to nest them? Is that... sensible?

Well there isn't anything inherently bad about nested nix-shells in that we are only fiddling with ENV variables really and i've certainly done that before without noticing any issues (obviously it's even better when multiple shells aren't required at all if the evaluation time is noticable but we can't avoid this here anyway).

I could also remove the outer nix-shell from the buildkite script and we then just run execute everything through individual nix-shell --run invocations in the ci-plutus-benchmark.sh script.

I'm really fine either way, no big issue to adjust it!

@michaelpj
Copy link
Contributor

I have no particular worries, just vague suspicion. We'd need some kind of nix-shell for cabal, so it seems okay.

@michaelpj michaelpj merged commit 8c42e1f into master Jul 1, 2021
@kwxm kwxm deleted the ci-benchmark-shells branch October 12, 2021 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelpj michaelpj Awaiting requested review from michaelpj

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gilligan @michaelpj