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.

Sign up for GitHub

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

Jump to bottom

veroroute: init at 2.39 #371217

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

veroroute: init at 2.39 #371217

wants to merge 1 commit into from

Conversation

zeuner
Copy link
Contributor

@zeuner zeuner commented Jan 5, 2025

Adding veroroute, a Qt based Veroboard, Perfboard, and PCB layout and routing application.

Apparently, #369345 has been uploaded since I started working on this. Uploading my version anyway since the other PR fails on CI.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Defelo
Defelo reviewed Jan 5, 2025
View reviewed changes
pkgs/by-name/ve/veroroute/package.nix Outdated
Comment on lines 33 to 43
meta = with lib; {
homepage = "https://sourceforge.net/projects/veroroute/";
description = "Qt based electronics layout and routing application";
longDescription = ''
Qt based Veroboard, Perfboard, and PCB layout and routing application
'';
license = licenses.gpl3Plus;
platforms = platforms.unix;
maintainers = with maintainers; [ gm6k ];
};
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
meta = with lib; {
homepage = "https://sourceforge.net/projects/veroroute/";
description = "Qt based electronics layout and routing application";
longDescription = ''
Qt based Veroboard, Perfboard, and PCB layout and routing application
'';
license = licenses.gpl3Plus;
platforms = platforms.unix;
maintainers = with maintainers; [ gm6k ];
};
meta = {
homepage = "https://sourceforge.net/projects/veroroute/";
description = "Qt based electronics layout and routing application";
longDescription = ''
Qt based Veroboard, Perfboard, and PCB layout and routing application
'';
license = lib.licenses.gpl3Plus;
platforms = lib.platforms.unix;
maintainers = with lib.maintainers; [ gm6k ];
};

zeuner
zeuner commented Jan 7, 2025
View reviewed changes
pkgs/by-name/ve/veroroute/package.nix
Comment on lines +24 to +25
qmake
wrapQtAppsHook
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing and for the suggestion. Could you elaborate on why this would be better, as I don't quite understand it. I think there is no straightforward reason for importing qmake and wrapQtAppsHook from different Qt versions, so it's probably better to use the inherit clause above, which imports both from the same Qt version. The older way to do this was to use libsForQt5.callPackage, but this would require an entry in pkgs/top-level/all-packages.nix, which is to be avoided. The inherit clause I used is as near to the old way as I could get without needing to touch another file.

zeuner
zeuner commented Jan 7, 2025
View reviewed changes
pkgs/by-name/ve/veroroute/package.nix
Comment on lines +8 to +10
let
inherit (qt5) qmake wrapQtAppsHook;
in
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See below, regarding libsForQt5.

zeuner
zeuner commented Jan 7, 2025
View reviewed changes
pkgs/by-name/ve/veroroute/package.nix
Comment on lines +36 to +38
longDescription = ''
Qt based Veroboard, Perfboard, and PCB layout and routing application
'';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing and for the suggestion. Could you elaborate on why this would be better, as I don't quite understand it. I think this information might be useful for potential users to decide whether the supported boards make the software useful for their layout and routing purposes.

zeuner
zeuner commented Jan 7, 2025
View reviewed changes
pkgs/by-name/ve/veroroute/package.nix
];

postInstall = ''
mkdir $out/bin
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This breaks the package, as it results in $out/bin being a file instead of a directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Defelo Defelo Defelo left review comments

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zeuner @Defelo