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.

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

colstr: init at 1.0.0 #276491

Merged
merged 1 commit into from Apr 10, 2024
Merged

colstr: init at 1.0.0 #276491

merged 1 commit into from Apr 10, 2024

Conversation

auroraanna
Copy link
Contributor

@auroraanna auroraanna commented Dec 24, 2023

Description of changes

Added colstr package.

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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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.

@auroraanna
Copy link
Contributor Author

Result of nixpkgs-review run on x86_64-linux 1

1 package built:
  • colstr

Copy link
Member

@donovanglover donovanglover left a comment

Choose a reason for hiding this comment

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

Works for me although it may be possible to eliminate the buildPhase (any C++ experts?)

pkgs/by-name/co/colstr/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/co/colstr/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/co/colstr/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/co/colstr/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/co/colstr/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/co/colstr/package.nix Outdated Show resolved Hide resolved
};

nativeBuildInputs = [ gcc ];
buildPhase = ''
Copy link
Member

Choose a reason for hiding this comment

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

I think there should be a way to get rid of the g++ step here. Then you can change buildPhase to installPhase when copying the binary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

split up the phases now…
How would you remove the compilation command? There is no makefile.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm I suppose it's fine as-is. perseus was the only other package I could find that has a g++ buildPhase

pkgs/by-name/co/colstr/package.nix Show resolved Hide resolved
pkgs/by-name/co/colstr/package.nix Outdated Show resolved Hide resolved
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/1358

@AndersonTorres
Copy link
Member

Please rebase

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/1564

@auroraanna
Copy link
Contributor Author

yup, did that.

Co-authored-by: Donovan Glover <donovan@dglover.co>
Co-authored-by: Anderson Torres <torres.anderson.85@protonmail.com>
Copy link
Member

@donovanglover donovanglover left a comment

Choose a reason for hiding this comment

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

Result of nixpkgs-review pr 276491 run on x86_64-linux 1

1 package built:
  • colstr

@wegank wegank merged commit 0514630 into NixOS:master Apr 10, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants