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

iverilog: 2017.08.12 -> 2018.12.15 #52994

Merged
merged 1 commit into from Dec 28, 2018
Merged

iverilog: 2017.08.12 -> 2018.12.15 #52994

merged 1 commit into from Dec 28, 2018

Conversation

andrey-komarov
Copy link
Contributor

Motivation for this change

Bump version to current master. The old version seems to also be "current" master because it's not tagged.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@thoughtpolice
Copy link
Member

@GrahamcOfBorg build verilog

veprbl
veprbl reviewed Dec 28, 2018
View reviewed changes
pkgs/applications/science/electronics/verilog/default.nix
@@ -2,13 +2,13 @@

stdenv.mkDerivation rec {
name = "iverilog-${version}";
version = "2017.08.12";
version = "2018.12.15";
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
version = "2018.12.15";
version = "unstable-2018.12.15";

https://nixos.org/nixpkgs/manual/#sec-package-naming

Copy link
Member

Choose a reason for hiding this comment

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

Not that there appear to be any consistency within Nixpkgs but shouldn't unstable be part of (p)name to more closely reflect the manual guidelines?

Copy link
Member

Choose a reason for hiding this comment

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

(And it kind of renames an existing package, if not its attribute name?..)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this would also break any lexicographic sorting/version comparisons on .version as well (builtins.compareVersions, etc). Such a rename also will break people who have installed by the existing attribute name.

But I don't think -unstable needs to be part of the name in any case (most of the other EDA-oriented expressions, many of which I maintain, use this same convention without -unstable). This is fine to merge.

@thoughtpolice thoughtpolice merged commit bc7aa5d into NixOS:master Dec 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thoughtpolice thoughtpolice thoughtpolice left review comments

@pbogdan pbogdan pbogdan left review comments

@veprbl veprbl veprbl left review comments

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

Successfully merging this pull request may close these issues.

None yet
5 participants
@andrey-komarov @thoughtpolice @pbogdan @veprbl @GrahamcOfBorg