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

Replace current CI implementation with Nix-flake based CI #97

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

notgne2
Copy link

@notgne2 notgne2 commented May 2, 2022

Description

We have newer practices for CI systems, so we should probably utilize them here, including our newest addition: mkPipeline, which generates individual pipeline steps for CI out of your flake, as opposed to lumping them all into a nix flake check step.

Related issue(s)

#26

✅ Checklist for your Pull Request

Related changes (conditional)

  • Tests

    • If I added new functionality, I added tests covering it.
    • If I fixed a bug, I added a regression test to prevent the bug from
      silently reappearing again.
  • Documentation

    • I checked whether I should update the docs and did so if necessary:

Stylistic guide (mandatory)

@notgne2 notgne2 requested a review from MagicRB May 2, 2022 23:57
@notgne2
Copy link
Author

notgne2 commented May 3, 2022

(might take some messing around with, this is my first time setting something up with mkPipeline)

@notgne2
Copy link
Author

notgne2 commented May 3, 2022

that seems to all be working except for weeder, which fails with

weeder: user error (Didn't find exactly 1 cabal file in .)

I might leave figuring that out to somebody that knows more about cabal

@notgne2 notgne2 marked this pull request as draft May 3, 2022 04:03
@notgne2
Copy link
Author

notgne2 commented May 3, 2022

Setting as draft until serokell/serokell.nix#69 is addressed, then will need to clean up commits

@dcastro
Copy link
Member

dcastro commented May 3, 2022

I noticed that we're missing a few steps we used to have (there may be more):

  • golden tests
  • validate cabal files
  • stylish
  • lint (there is a new hlilnt step now, but looking at the logs it doesn't seem like it's actually running hlint)
  • shellcheck
  • linux static executable
  • xrefcheck

Also:

  • the tests step seems to be building the tests component, but not actually running it
  • same for the haddock step
  • same for the doctests step (In fact, I had issues trying to get doctests to run correctly on the CI before. So if we can't get it to run and pass, we might as well just ignore it in the CI, which is what we've been doing so far).

Questions:

  • How do I run these CI steps locally? Previously, I was able to copy/paste the commands from .buildkite/pipeline.yml. But I'm not familiar with flakes, so I wouldn't know how to reproduce this locally.

@notgne2
Copy link
Author

notgne2 commented May 3, 2022

I'll add in the missing test-runnig and try to add all the missing tests too

How do I run these CI steps locally?

With using mkPipeline, it's generating our pipeline steps from what is under packages and checks, the same things that will be built by Nix if you run nix flake check.

@notgne2
Copy link
Author

notgne2 commented May 3, 2022

Adding xrefcheck would be ugly unless we can first deal with serokell/xrefcheck#54 (which would probably ending up looking exactly like this PR)

@notgne2
Copy link
Author

notgne2 commented May 7, 2022

It also seems like "validate cabal files" might be tedious to add, I tried it and it failed while attempting network access, is this because I configured it wrong? or is that step just impure

@dcastro
Copy link
Member

dcastro commented May 7, 2022

It also seems like "validate cabal files" might be tedious to add, I tried it and it failed while attempting network access, is this because I configured it wrong? or is that step just impure

Looks like it's impure. The script validate-cabal-files.sh calls stack2cabal, which I think needs to fetch the stack resolver.

@notgne2 notgne2 force-pushed the notgne2/improve-ci branch from 2f94b32 to ac3bc48 Compare June 7, 2022 14:10
@notgne2 notgne2 force-pushed the notgne2/improve-ci branch 3 times, most recently from 71a1b4d to bd692fa Compare June 7, 2022 14:29
@notgne2 notgne2 force-pushed the notgne2/improve-ci branch from bd692fa to f5e7977 Compare June 7, 2022 15:01
@notgne2 notgne2 force-pushed the notgne2/improve-ci branch from 9b130c0 to b67a0af Compare June 7, 2022 15:08
@notgne2 notgne2 marked this pull request as ready for review June 7, 2022 15:09
@notgne2 notgne2 requested a review from a team June 7, 2022 15:10
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.

2 participants