-
Notifications
You must be signed in to change notification settings - Fork 12
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
Housekeeping #40
Housekeeping #40
Conversation
7744749
to
d47c7a1
Compare
Hi Spike! Thanks for this contribution! Add
|
94323a4
to
f75461c
Compare
|
Here we go. I was going to leave it alone and just accept that people like Alejandra, but it does cause problems. The reason I dislike Alejandra is because of what I just dealt with: Alejandra causes merge conflicts, because it tends to change a lot of code for even small changes. It is also not strict enough (like I tried to squash |
88f4c74
to
1fc952d
Compare
As for
Imagine a flake only wants to export packages for platforms it officially supports. Such as, Now, a user comes along using All they have to do in their flake is this:
Where they have their own
That file might contain:
You don't have to edit the flake to add those two platforms, they don't have to fork it, everyone is happy. They know it isn't officially supported, because you didn't export that as your default For me, a practical example would be adding
I'd like to point that out as a bad idea... You should want to be precise here, not just brute-force a solution to the problem.
You don't need to do that, because the user can simply override
No, that is not what overlays are for. Overlays simply allow you to merge packages or attributes into the top-level Someone might want to stay with the All of that said, I notice a mistake with the way I did this. I should not be using To appease you however, I will not use This does have a downside: the My recommendation is using New commit: ce46d1a |
You might also want |
I think that adding all of the packages as Edit: The Nix manual says it checks that the packages are derivations, but it doesn't say anything about running What it does do is I found your issue: NixOS/nix#8882 It doesn't say anything about building packages, but:
|
1fc952d
to
48fe5d2
Compare
I have added a formatting check, as you requested. Took me a while to figure out how to do it, but now I know for my own projects in the future. |
I'll have to check out |
Could you please re-run the CI? |
I'm seeing |
13440df
to
5ac7215
Compare
Because formatters may wrap long lines with comments, I have replaced `LOAD-BEARING COMMENT` on the package's `version` attribute with `@VERSION@`. This was done so that `sed` can still match the line, because it is short enough to not wrap at 80 columns.
1. Move custom check commands to `preCheck` instead of `postCheck`, because source code checks should be run *before* `cargo test` (which is what the defualt `checkPhase` for `buildRustPackage` does). 2. Remove `cargo fmt --check` because the derivation should still build regardless of formatting. 3. Add `cargo check --frozen` because the package should not be used if this does not pass, in addition to the existing Clippy command. 4. Remove the `&& echo` parts of the check commands, because they cause the check to always succeed. There will already be output if these commands fail, which is what we *actually* care about as opposed to if they were successful.
5ac7215
to
8cb55b1
Compare
Changed to use |
Thank you! |
Any of these commits may be dropped or rebased should you disapprove.
I recommend reviewing this PR by individual commit because of the diff-noise produced by the formatter (last commit).
meta.mainProgram
(and othermeta
attributes) to the package.af3c6a8
nil
lint about unused function argumentflake-compat
, replace with...
.0796022
rec
frompackages
output, prefer referencing throughself
for package aliases.30ebc49
system
fromimport nixpkgs
arguments, preferlocalSystem
.82a5d24
system
argument has been referred to as "legacy" since 2017.stdenv
: https://github.com/NixOS/nixpkgs/blob/5d2b2523a0442fd19294359b31b8535696c32984/pkgs/stdenv/default.nix#L10flake-utils
.61d2ab4
flake-utils
from 2022 is: "It's a stable mess."flake-utils
.898524a
systems
overrideable through usage ofnix-your-shell.inputs.systems
.nixfmt
as the flake'sformatter
.d47c7a1
nixfmt
with members of the NixOS organization about "blessing" this as the canon formatter. Unfortunately, contrary to the style preferred by many individual flake projects, they decided not to go through with it and instead are "waiting" fornixpkgs-fmt
to be "ironed out".sed
script and theLOAD-BEARING COMMENT
to make it shorter, so thatnixfmt
doesn't wrap the line. Sincesed
can't do multiline matching replacement easily, this was the solution that resisted me the least. I recommend taking a look at its replacement, and advising me further if need-be. No matter the flake'sformatter
, making this mechanism less fragine would be a good idea.I will also look into making another PR down the line (once I figure it out for myself) about simplifying the
version.yaml
workflow. This makes me uneasy, having ased
script target a comment. Ideally, that version should be based on the flake's revision, however I do understand it is necessary to be the way it is because of publishing to crates.io.