Skip to content

colobot: init at 0.1.12-alpha #85419

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

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.

Sorry, something went wrong.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Apr 17, 2020
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
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 1, 2020
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 1, 2020
@ryantm ryantm added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 3, 2020
@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

@SuperSandro2000 SuperSandro2000 May 10, 2021

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
Member

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.

@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants