-
-
Notifications
You must be signed in to change notification settings - Fork 17.5k
qwertone: init at 0.5.0 #388974
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
qwertone: init at 0.5.0 #388974
Conversation
pkgs/by-name/qw/qwertone/package.nix
Outdated
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.
| description = "Music synthesizer"; | |
| description = "Simple music synthesizer app based on usual qwerty-keyboard for input"; |
ethancedwards8
left a comment
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.
LGTM. Nice work.
| rustPlatform.buildRustPackage rec { | ||
| pname = "qwertone"; | ||
| version = "0.5.0"; | ||
|
|
||
| src = fetchFromGitLab { | ||
| domain = "gitlab.com"; | ||
| owner = "azymohliad"; | ||
| repo = "qwertone"; | ||
| tag = "v${version}"; |
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.
| rustPlatform.buildRustPackage rec { | |
| pname = "qwertone"; | |
| version = "0.5.0"; | |
| src = fetchFromGitLab { | |
| domain = "gitlab.com"; | |
| owner = "azymohliad"; | |
| repo = "qwertone"; | |
| tag = "v${version}"; | |
| rustPlatform.buildRustPackage (finalAttrs: { | |
| pname = "qwertone"; | |
| version = "0.5.0"; | |
| src = fetchFromGitLab { | |
| domain = "gitlab.com"; | |
| owner = "azymohliad"; | |
| repo = "qwertone"; | |
| tag = "v${finalAttrs.version}"; |
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.
If this really useful? When overriding the version, the whole src and cargoDeps need to be overrided.
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.
It is considered a best practice as it also removes the need for rec, but I will not block you on it
(especially not blocking because buildRustPackage only recently got the ability to use finalAttrs, so there might a treewide PR eventually as many packages still use rec for the version)
niklaskorz
left a comment
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.

Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.