Skip to content

2024 edition update and other minor refactors#199

Merged
piegamesde merged 1 commit intoandir:masterfrom
Coca162:minor-refactors
Feb 11, 2026
Merged

2024 edition update and other minor refactors#199
piegamesde merged 1 commit intoandir:masterfrom
Coca162:minor-refactors

Conversation

@Coca162
Copy link
Collaborator

@Coca162 Coca162 commented Jan 31, 2026

Some cleanup work for the codebase:

@Coca162
Copy link
Collaborator Author

Coca162 commented Jan 31, 2026

Seems like a spurious CI failure(?)

@Coca162
Copy link
Collaborator Author

Coca162 commented Feb 1, 2026

Switched to a single threaded runtime for tokio, shouldn't have any impact but just minor efficiency gains

@piegamesde
Copy link
Collaborator

Switched to a single threaded runtime for tokio, shouldn't have any impact but just minor efficiency gains

Please don't, AFAIK single-threaded runtimes can quite easily deadlock when not being careful about it, and we're not performance critical anyways so the overhead should be negligible

@Coca162
Copy link
Collaborator Author

Coca162 commented Feb 1, 2026

Please don't, AFAIK single-threaded runtimes can quite easily deadlock when not being careful about it, and we're not performance critical anyways so the overhead should be negligible

I could only really imagine that stdout/stderr locking in the updating/verifying code would be a cause for worry here, which would hopefully be caught easily in testing before it could become a issue . Even if we don't switch to single threading I would prefer to have only 2 threads for the sake of not being excessive.

@piegamesde piegamesde mentioned this pull request Feb 1, 2026
5 tasks
@Coca162
Copy link
Collaborator Author

Coca162 commented Feb 1, 2026

Since it's a relatively minor change code wise anyhow, I'll spin the single threaded tokio runtime off into another PR, I'll keep testing it in the meantime to see if I get any weird edge cases with my use.

@Coca162 Coca162 force-pushed the minor-refactors branch 2 times, most recently from daf7155 to 51c3829 Compare February 2, 2026 21:54
@Coca162
Copy link
Collaborator Author

Coca162 commented Feb 2, 2026

Added back the comment about clap's version that was accidentally stripped out with #186, I assume it still applies so

@Coca162 Coca162 force-pushed the minor-refactors branch 3 times, most recently from 9be3b49 to 0539b0d Compare February 3, 2026 21:17
@Coca162
Copy link
Collaborator Author

Coca162 commented Feb 3, 2026

Not sure about why the CI is failing on snix now, locally I can run it fine with this hash...

@andir
Copy link
Owner

andir commented Feb 5, 2026

Not sure about why the CI is failing on snix now, locally I can run it fine with this hash...

IIRC snix has some store paths in their files and those will be reference scanned and then nix complains about undeclared / impure references. I'm not sure why that doesn't surface for you locally, but I can reproduce it on my machines.

Perhaps it is best to just delay the snix package upgrade, if we can.

@Coca162 Coca162 force-pushed the minor-refactors branch 2 times, most recently from 9c14367 to 98988bf Compare February 5, 2026 11:44
@Coca162
Copy link
Collaborator Author

Coca162 commented Feb 5, 2026

Pinned it to the revision we are using already in the manifest

- Using edition 2024 for all crates
- Fixed all checks/clippy warnings
- [`clippy::wildcard_imports`](https://rust-lang.github.io/rust-clippy/stable/index.html#wildcard_imports) now warns for libnpins
- `anyhow::Result` is now always fully qualified in libnpins
- [Formatting code with the new edition](https://doc.rust-lang.org/edition-guide/rust-2024/rustfmt-formatting-fixes.html)
- `cargo update`
-  Turn off default features for `nix-compat` and set exact revision
@piegamesde piegamesde merged commit 3c13797 into andir:master Feb 11, 2026
1 check passed
@Coca162 Coca162 deleted the minor-refactors branch February 11, 2026 13:39
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.

3 participants