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 improvments #93

Merged
merged 5 commits into from
Jul 30, 2023
Merged

Flake improvments #93

merged 5 commits into from
Jul 30, 2023

Conversation

sbatial
Copy link
Collaborator

@sbatial sbatial commented Jul 30, 2023

Tracking issue: #72

  • remove a bunch of whitespace
  • check nix files' formatting
  • run test suite with nix flake check
  • run cargo test when building via flake to ensure working output
  • add clippy as part of the toolchain

@sbatial sbatial self-assigned this Jul 30, 2023
@cafkafk cafkafk self-requested a review July 30, 2023 13:15
Copy link
Member

@cafkafk cafkafk left a comment

Choose a reason for hiding this comment

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

Nothing blocking, just comments.

Comment on lines -17 to -22
self
, flake-utils
, naersk
, nixpkgs
, treefmt-nix
, rust-overlay
Copy link
Member

Choose a reason for hiding this comment

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

I actually prefer this, it's the same style that https://github.com/nixos/nixpkgs uses.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There seems to already have been a lot of discussion on the topic: kamadorueda/alejandra#95

I don't care that much at the end of the day. I use alejandra because I found it's formatting pretty helpful but unintrusive working with it. (Style discussions are some of the most annoying things devs - let's be honest - fight about imo)

Judging by context I assume you don't insist on using something else?!

@@ -1,8 +1,7 @@
{
projectRootFile = "Cargo.toml";
programs = {
# alejandra.enable = true;
alejandra.enable = true;
Copy link
Member

Choose a reason for hiding this comment

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

I don't know this package, but I'm gonna check it out!

Copy link
Member

@cafkafk cafkafk left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@sbatial
Copy link
Collaborator Author

sbatial commented Jul 30, 2023

Regarding #75: I went looking for the tests because they did not show up here but I think that might be because they aren't yet in the tree of this branch (?! idk)

edit: ah no. i think it's because the paths aren't in there. makes sense

@sbatial sbatial merged commit f717a03 into main Jul 30, 2023
@cafkafk
Copy link
Member

cafkafk commented Jul 30, 2023

Regarding #75: I went looking for the tests because they did not show up here but I think that might be because they aren't yet in the tree of this branch (?! idk)

Sounds plausible, but annoying. Maybe I'll try merging main into a PR and see if it triggers CI.

@sbatial sbatial deleted the flake-improvments branch July 30, 2023 13:38
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