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

colobot: init at 0.1.12-alpha #85419

Closed
wants to merge 4 commits into from
Closed

colobot: init at 0.1.12-alpha #85419

wants to merge 4 commits into from

Conversation

wesnel
Copy link
Member

@wesnel wesnel commented Apr 17, 2020

Motivation for this change

I've added a derivation for Colobot, a GPL-licensed educational programming game. Thanks for looking!

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 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

pkgs/games/colobot/default.nix Outdated Show resolved Hide resolved
pkgs/games/colobot/default.nix Outdated Show resolved Hide resolved
pkgs/games/colobot/default.nix Outdated Show resolved Hide resolved
Wesley Nelson and others added 2 commits April 18, 2020 05:36
colobot: remove vorbisTools argument

colobot: reduce scope of `colobotMusic`

colobot: use cmake flag instead of using sed

colobot: clean up regex
@stale
Copy link

stale bot commented Apr 7, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 7, 2021
@applePrincess
Copy link
Contributor

merge commits adding you to maintainers into a single commit with a message "maintainers: add wesnel", and other commits for adding colobot package into single commit with a commit message "colobot: init at 0.1.12"

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label May 9, 2021
Comment on lines +176 to +177

colobot = callPackage ../games/colobot { };
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
colobot = callPackage ../games/colobot { };
colobot = callPackage ../games/colobot { };

tar -xvf ${colobotMusic} -Cdata/music
'';

meta = with stdenv.lib; {
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 stdenv.lib; {
meta = with lib; {

stdenv.lib is deprecated.

Comment on lines +76 to +77
sed -i 's/find_program(WGET wget)//' \
data/music/CMakeLists.txt
Copy link
Member

Choose a reason for hiding this comment

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

Please use substituteInPlace.

Comment on lines +67 to +68
url =
"https://colobot.info/files/music/colobot-music_ogg_${version}.tar.gz";
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
url =
"https://colobot.info/files/music/colobot-music_ogg_${version}.tar.gz";
url = "https://colobot.info/files/music/colobot-music_ogg_${version}.tar.gz";

cmakeFlags = [ "-DCOLOBOT_INSTALL_BIN_DIR=${placeholder "out"}/bin" ];

postPatch = let
colobotMusic = fetchurl {
Copy link
Member

Choose a reason for hiding this comment

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

Please move that into a let in before mkDerivation.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO using narrower scopes is better.

gettext
librsvg
perl
python27
Copy link
Member

Choose a reason for hiding this comment

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

Can we use python3?

@stale
Copy link

stale bot commented Nov 9, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 9, 2021
@FliegendeWurst
Copy link
Member

PR #103077 added the same package.

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

7 participants