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.

Sign up for GitHub

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

Jump to bottom

mindustry: init at 99 (and updates) #72306

Merged
merged 5 commits into from
Jan 13, 2020
Merged

mindustry: init at 99 (and updates) #72306

merged 5 commits into from
Jan 13, 2020
+108 −0

Conversation

fgaz
Copy link
Member

@fgaz fgaz commented Oct 30, 2019

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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.
Notify maintainers

cc @

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: 1-10 10.rebuild-linux: 1-10 labels Oct 30, 2019
@ghost ghost mentioned this pull request Nov 1, 2019
Closed
10 tasks
@wamserma
Copy link
Member

wamserma commented Nov 3, 2019

LGTM. Tested on NixOS 19.09.

@wamserma wamserma mentioned this pull request Nov 3, 2019
Closed
@ghost
Copy link

ghost commented Nov 3, 2019

LGTM

@deliciouslytyped deliciouslytyped mentioned this pull request Nov 4, 2019
Closed
@fgaz fgaz force-pushed the mindustry/init branch 2 times, most recently from 7f195dc to 5e834f1 Compare November 9, 2019 10:53
@fgaz
Copy link
Member Author

fgaz commented Nov 25, 2019

v100 is out, I just pushed an update. Is there anything blocking this?

@fgaz
Copy link
Member Author

fgaz commented Dec 13, 2019

Updated to 101

@fgaz
Copy link
Member Author

fgaz commented Dec 22, 2019

Updated to 101.1

...yes, I'll continue to ping everyone here at every release until this gets merged or rejected ;-)

@jappeace
Copy link
Contributor

please merge this

@fgaz
Copy link
Member Author

fgaz commented Jan 1, 2020

Updated to 102 and rebased

@fgaz fgaz changed the title mindustry: init at 99 mindustry: init at 99 (and updates) Jan 1, 2020
@greizgh
Copy link
Contributor

greizgh commented Jan 5, 2020

Built and tested successfully on x86_64

@davidak davidak mentioned this pull request Jan 10, 2020
Closed
ghost
ghost approved these changes Jan 10, 2020
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I think this package has above-average quality and can be merged. I have built it using sandboxing on NixOS x86_64 and tested the basic functionality.

  • One small wish/suggestion would be to package the server as a seperate derivation, to prevent pulling in pulseaudio on a headless machine, but I can also propose the change after it was merged.

  • Also I noticed it does not seem to like SDL_VIDEODRIVER=wayland, which I have set in my environment, but this is a problem for upstream. It might be good to add set SDL_VIDEODRIVER to x11 explicitly in the wrapper for users who have set it to wayland in their environment (because most applications just work with it).

    [I] Initialized Discord rich presence.
arc.backend.sdl.SDLError: wayland not available
	at arc.backend.sdl.SdlApplication.check(SdlApplication.java:181)
	at arc.backend.sdl.SdlApplication.init(SdlApplication.java:80)
	at arc.backend.sdl.SdlApplication.<init>(SdlApplication.java:30)
	at mindustry.desktop.DesktopLauncher.main(DesktopLauncher.java:46)
[I] Sending crash report.
[I] Crash sent successfully.

    This is not critical though because every person who has set this variable manually in their setup will know what to do from the error message.

  • You may want to squash all of those commits together.

@peterhoeg peterhoeg merged commit 36f084b into NixOS:master Jan 13, 2020
dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Jan 13, 2020
@fgaz @dtzWill
mindustry: init at 102 (and updates) (NixOS#72306)
e1bab75 
* mindustry: init at 99

* mindustry: 99 -> 100

* mindustry: 100 -> 101

* mindustry: 101 -> 101.1

* mindustry: 101.1 -> 102

(cherry picked from commit 36f084b)
@fgaz fgaz mentioned this pull request Jan 17, 2020
Closed
@fgaz
Copy link
Member Author

fgaz commented Jan 17, 2020

@petabyteboy
I reported the wayland issue upstream: Anuken/Mindustry#1393
Splitting it is a good idea. Opened #77893 to track it.

@fgaz fgaz deleted the mindustry/init branch January 26, 2020 15:17
@fgaz fgaz mentioned this pull request Jan 26, 2020
Merged
10 tasks
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/january-2020-in-nixos/5771/1

@lukegb lukegb mentioned this pull request Nov 27, 2020
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tadfisher tadfisher

@chkno chkno

Assignees
No one assigned
Labels
6.topic: games 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@fgaz @wamserma @jappeace @greizgh @nixos-discourse @tadfisher @chkno @peterhoeg