-
Notifications
You must be signed in to change notification settings - Fork 11
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
project: deprecate 8.10.7 #254
Conversation
WalkthroughThe changes involve modifications to four files: Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Nix
participant GHC
participant DevTools
User->>Nix: Request development environment
Nix->>GHC: Setup GHC 9.6.6
Nix->>DevTools: Include myDevTools
DevTools-->>Nix: Tools ready
Nix-->>User: Development environment ready
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
flake.nix (1)
15-27
: Consider documenting the cabal-install comment
The development tools list is comprehensive and well-structured. However, the commented-out cabal-install
line could use a comment explaining why it's disabled, especially since both Stack and Cabal builds were tested according to the PR description.
- # hPkgs.cabal-install
+ # hPkgs.cabal-install # Disabled as we're using stack-wrapped instead
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
flake.lock
is excluded by!**/*.lock
📒 Files selected for processing (3)
- flake.nix (1 hunks)
- klister.cabal (1 hunks)
- stack.yaml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- klister.cabal
🔇 Additional comments (7)
stack.yaml (3)
6-7
: LGTM: Nix integration looks good
The addition of Nix support with enable: true
is appropriate and aligns with the PR objective of making the nix flake work with Stack.
1-1
: Verify dependency compatibility with LTS 22.39
The major version bump from LTS 20.25 to 22.39 represents a significant change. While this aligns with the goal of updating to GHC 9.6.x, we should verify that all project dependencies are compatible with this new resolver.
✅ Verification successful
LTS 22.39 resolver update appears compatible
The dependency version bounds in the cabal file are compatible with LTS 22.39:
- All version bounds are sufficiently flexible
- No restrictive upper bounds that would conflict with GHC 9.6.x
- Empty
extra-deps
section suggests all dependencies are satisfied by the resolver
The update from LTS 20.25 to LTS 22.39 should work without issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the project builds with the new resolver
# and if there are any version bounds issues in the cabal file
# Look for any version bounds in cabal file that might conflict with LTS 22.39
rg -A 3 "build-depends:" "*.cabal"
Length of output: 82
Script:
#!/bin/bash
# Let's first find the cabal file(s) in the repository
fd -e cabal
# Then check the package.yaml as an alternative since this might be a hpack project
fd "package.yaml"
Length of output: 45
Script:
#!/bin/bash
# Now that we found the cabal file, let's examine its dependencies
cat klister.cabal | rg -A 5 "build-depends:"
# Also, let's check what versions are available in the new resolver
# by examining the stack.yaml file for any extra-deps or other configurations
cat stack.yaml
Length of output: 595
4-5
: Verify removed dependencies are available in LTS 22.39
The following packages were removed from extra-deps:
- transformers-0.6.1.0
- mtl-2.3.1
- exceptions-0.10.7
- hedgehog-1.2
- tasty-hedgehog-1.4.0.0
Please confirm these are available in the new resolver's package set.
✅ Verification successful
Removed dependencies are correctly available in LTS 22.39
All previously removed dependencies from extra-deps
are now properly specified in the cabal file with version constraints that are compatible with LTS 22.39:
transformers ^>= 0.6
(in common deps)mtl >= 2.2.2 && < 2.4
(in common deps)exceptions
(in common deps)hedgehog >= 1.2 && < 1.5
(in test suite)tasty-hedgehog >= 1.4 && < 1.5
(in test suite)
Since these packages are standard dependencies available in the new resolver, they no longer need to be listed in extra-deps
. The removal is correct.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if removed dependencies are available in new resolver
# Look for these package dependencies in the cabal file
rg "transformers|mtl|exceptions|hedgehog" "*.cabal"
Length of output: 101
Script:
#!/bin/bash
# First, let's find the cabal file(s) in the repository
fd -e cabal
# Then, let's check stack.yaml for the resolver version
cat stack.yaml
Length of output: 111
Script:
#!/bin/bash
# Let's check the dependencies in the cabal file
cat klister.cabal
Length of output: 3171
flake.nix (4)
11-13
: LGTM! Clear version specification with helpful comment
The GHC version is explicitly set to match the Stackage LTS version, which is good practice for maintaining consistency.
29-45
: LGTM! Well-documented Stack wrapper configuration
The Stack wrapper configuration is excellent, with clear comments explaining each flag's purpose. This ensures consistent behavior and proper Nix integration across the team.
48-54
: LGTM! Proper shell environment configuration
The shell environment is well-configured with:
- Comprehensive development tools inclusion
- Proper C library integration via LD_LIBRARY_PATH
- Clear documentation with upstream reference
2-3
: Verify the nixos-unstable channel compatibility
The change from nixpkgs-unstable
to nixos-unstable
channel is notable. While nixos-unstable
is generally more stable, please verify that all required Haskell packages are available in this channel.
✅ Verification successful
The nixos-unstable channel is compatible with the GHC 9.6.6 upgrade
The verification shows that:
- GHC 9.6.6 is properly referenced in the flake configuration
- All required Haskell development tools (ghcid, ormolu, hlint, hoogle, haskell-language-server, implicit-hie, retrie) are available through the specified channel
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if GHC 9.6.6 and other required packages are available in nixos-unstable
# Test: Search for GHC 9.6.6 package
rg -A 5 "ghc966"
# Test: Check if all Haskell packages specified in myDevTools are available
rg "ghcid|ormolu|hlint|hoogle|haskell-language-server|implicit-hie|retrie"
Length of output: 953
fd388fb
to
448bc1a
Compare
ba8b60e
to
ae9074c
Compare
Once Ci finishes #223 can be rebased |
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.
I don't know anything about nix flakes, but I approve dropping support for ghc-8.10!
Ah yea I changed the flake so that it creates the right developer environment and allows the use of |
Made the nix flake work with stack. This bumps the stack lts. I built with both stack and cabal and it seemed fine.
Summary by CodeRabbit
New Features
Bug Fixes
Chores