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

nottetris2: init at 2.0 #87028

Merged
merged 3 commits into from May 22, 2020
Merged

nottetris2: init at 2.0 #87028

merged 3 commits into from May 22, 2020

Conversation

@Sohalt
Copy link
Contributor

Sohalt commented May 6, 2020

Motivation for this change

I want to play this game

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.
@Sohalt
Copy link
Contributor Author

Sohalt commented May 6, 2020

The homepage does not state an explicit version, nor a specific license. I've used the same as in https://github.com/NixOS/nixpkgs/blob/master/pkgs/games/rimshot/default.nix which is from the same developers.

@Sohalt
Copy link
Contributor Author

Sohalt commented May 6, 2020

Turns out the source is actually also on GitHub with a proper version and license… https://github.com/Stabyourself/nottetris2/releases

@Sohalt Sohalt force-pushed the Sohalt:nottetris2 branch 2 times, most recently from 17f98d9 to 1c9c081 May 6, 2020
@Sohalt Sohalt changed the title nottetris2: init at 1.0 nottetris2: init at 2.0 May 6, 2020
@ajs124
Copy link
Member

ajs124 commented May 12, 2020

Please squash down to one commit.

@Sohalt Sohalt force-pushed the Sohalt:nottetris2 branch from b484496 to 2cc1514 May 18, 2020
@Sohalt
Copy link
Contributor Author

Sohalt commented May 18, 2020

Please squash down to one commit.

done

Copy link
Contributor

yorickvP left a comment

Nice! Was just about to do this

pkgs/games/nottetris2/default.nix Outdated Show resolved Hide resolved

meta = with stdenv.lib; {
description = "It's like Tetris, but it's not";
platforms = platforms.linux;

This comment has been minimized.

Copy link
@yorickvP

yorickvP May 21, 2020

Contributor

are we allowed to platforms = love_0_7.meta.platforms;?

This comment has been minimized.

Copy link
@Sohalt

Sohalt May 21, 2020

Author Contributor

Right now love_0_7.meta.platforms is stenv.lib.platforms.linux, so it would definitely work. It should most probably also work on darwin, should love2d get support for that, but I cannot test that.

Sohalt and others added 2 commits May 21, 2020
Co-authored-by: Yorick <yorickvanpelt@gmail.com>
@ofborg ofborg bot requested a review from yorickvP May 21, 2020
@yorickvP
Copy link
Contributor

yorickvP commented May 22, 2020

@GrahamcOfBorg build nottetris2

Copy link
Member

Ekleog left a comment

Apart from the source being difficult to validate as authoritative, this looks good to me :) hopefully it'd be easy to fetch from the zip file served from the official website?


src = fetchFromGitHub {
owner = "Stabyourself";
repo = pname;

This comment has been minimized.

Copy link
@Ekleog

Ekleog May 22, 2020

Member

Would it be possible to fetchurl from https://stabyourself.net/nottetris2/ instead? The website appears to be by far more reachable on the internet than the github when searching for nottetris2, and I can't find a link from the website to the github — the github being identity theft, while unlikely, is unfortunately always a possibility. This being said, I notice that orthorobot already uses the github, so… it's probably not that bad to add one dependency on this github here, but I wonder where the “authority” was taken from when merging orthorobot :)

This comment has been minimized.

Copy link
@ajs124

ajs124 May 22, 2020

Member

The download on the website isn't versioned and doesn't contain a license, so I'd be in favor of sticking with GitHub.

We can also do the direct approach: @Stabyourself you're one of the people responsible for stabyourself.net, right? Can you maybe include a link there to your GitHub or clarify the situation?

This comment has been minimized.

Copy link
@Stabyourself

Stabyourself May 22, 2020

Hi, yeah I'm the dev of the games. Back then I didn't really know/care much about open source standards, so everything's a bit home-baked, sorry about that.
I changed all the "Source" links on the website to go to github instead.

This comment has been minimized.

Copy link
@Ekleog

Ekleog May 22, 2020

Member

Great, thank you! 💯

Merging, as a consequence :)

@ofborg ofborg bot requested a review from yorickvP May 22, 2020
@Ekleog Ekleog merged commit 002d387 into NixOS:master May 22, 2020
19 checks passed
19 checks passed
editorconfig editorconfig
Details
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="29d0266"; rev="29d026699b3300b44f42102eb13a3b2ba22344cc"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="29d0266"; rev="29d026699b3300b44f42102eb13a3b2ba22344cc"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="29d0266"; rev="29d026699b3300b44f42102eb13a3b2ba22344cc"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="29d0266"; rev="29d026699b3300b44f42102eb13a3b2ba22344cc"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="29d0266"; rev="29d026699b3300b44f42102eb13a3b2ba22344cc"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="29d0266"; rev="29d026699b3300b44f42102eb13a3b2ba22344cc"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="29d0266"; rev="29d026699b3300b44f42102eb13a3b2ba22344cc"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
nottetris2 on aarch64-linux Success
Details
nottetris2 on x86_64-linux Success
Details
nottetris2, nottetris2.passthru.tests on aarch64-linux Success
Details
nottetris2, nottetris2.passthru.tests on x86_64-linux Success
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.