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

sameboy: init at 0.13.6 #98177

Merged
merged 2 commits into from Sep 19, 2020
Merged

sameboy: init at 0.13.6 #98177

merged 2 commits into from Sep 19, 2020

Conversation

meithecatte
Copy link
Contributor

Motivation for this change

Adds a new package.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • 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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@f4814
Copy link
Contributor

f4814 commented Sep 17, 2020

Tested it using nix-shell -p nixpkgs-review --run "nixpkgs-review pr 98177". Was able to successfully build the package and play some Super Mario :D

Copy link
Member

@IvarWithoutBones IvarWithoutBones left a comment

Choose a reason for hiding this comment

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

A few comments, otherwise LGTM 👍

maintainers/maintainer-list.nix Show resolved Hide resolved
pkgs/misc/emulators/sameboy/default.nix Outdated Show resolved Hide resolved
installPhase = ''
pushd build/bin/SDL
install -Dm755 sameboy $out/bin/sameboy
rm sameboy
Copy link
Member

Choose a reason for hiding this comment

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

There's no need to remove this manually, the build env gets garbage collected in its entirety when it's not required anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's not the role of this rm. The goal here is to avoid * matching the binary itself, copying only the rest. Should I add a comment to this effect? Or maybe there's a more idiomatic way of achieving this?

pkgs/misc/emulators/sameboy/default.nix Outdated Show resolved Hide resolved
@ehmry ehmry merged commit 220490a into NixOS:master Sep 19, 2020
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

4 participants