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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

thcrap-steam-proton-wrapper: init at 0-unstable-2024-04-03 #301845

Merged
merged 2 commits into from Apr 11, 2024

Conversation

ashuramaruzxc
Copy link
Member

@ashuramaruzxc ashuramaruzxc commented Apr 5, 2024

Description of changes

Add package with a wrapper script for running Touhou games under steam proton with:
thcrap patch (English localization, QOL and etc),
thprac (Practice tool),
vsync patch.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
1 package built:
thcrap-steam-proton-wrapper
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 馃憤 reaction to pull requests you find important.

@msanft
Copy link
Contributor

msanft commented Apr 5, 2024

Result of nixpkgs-review pr 301845 run on x86_64-linux 1

1 package built:
  • thcrap-steam-proton-wrapper

@msanft
Copy link
Contributor

msanft commented Apr 5, 2024

Also change the PR title format to thcrap-steam-proton-wrapper: init at 0-unstable-2024-04-03 please @ashuramaruzxc

@msanft
Copy link
Contributor

msanft commented Apr 5, 2024

Binary appears to work correctly to me

@ashuramaruzxc ashuramaruzxc changed the title init: thcrap-steam-proton-wrapper at 2b636c3 init: thcrap-steam-proton-wrapper at 0-unstable-2024-04-03 Apr 5, 2024
Copy link
Contributor

@msanft msanft left a comment

Choose a reason for hiding this comment

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

Only some nits, and adjust the PR title please

pkgs/by-name/th/thcrap-steam-proton-wrapper/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/th/thcrap-steam-proton-wrapper/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/th/thcrap-steam-proton-wrapper/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/th/thcrap-steam-proton-wrapper/package.nix Outdated Show resolved Hide resolved
@ashuramaruzxc ashuramaruzxc changed the title init: thcrap-steam-proton-wrapper at 0-unstable-2024-04-03 thcrap-steam-proton-wrapper: init at 0-unstable-2024-04-03 Apr 5, 2024
@DontEatOreo
Copy link
Member

DontEatOreo commented Apr 6, 2024

8e0c692 and 93efa69...99e296c are breaking Commit conventions

For 8e0c692

  • When adding yourself as maintainer in the same pull request, make a separate commit with the message maintainers: add <handle>.

For 93efa69...99e296c

  • If you have commits pkg-name: oh, forgot to insert whitespace: squash commits in this case. Use git rebase -i.

The last 4 commits: 93efa69...99e296c can be squashed into one and be reworded to thcrap-steam-proton-wrapper: apply suggestions from code review

Also reword 93efa69 to thcrap-steam-proton-wrapper: add long description

Edit: Ideally you could squash all commits (except 8e0c692) into 14a035d and reword 14a035d to thcrap-steam-proton-wrapper: init at 0-unstable-2024-04-03

Edit 2: It would also be nice if 8e0c692 is the first commit and 14a035d is second one

@ashuramaruzxc ashuramaruzxc force-pushed the package/thcrap-steam-proton-wrapper branch from 99e296c to 64ae837 Compare April 8, 2024 10:31
@github-actions github-actions bot added the 6.topic: stdenv Standard environment label Apr 8, 2024
@ashuramaruzxc
Copy link
Member Author

Okay, rebased and squashed commits, and reworded the commit about the version of the package and adding myself as maintainer.
@DontEatOreo

@ashuramaruzxc ashuramaruzxc force-pushed the package/thcrap-steam-proton-wrapper branch from 64ae837 to 03f2229 Compare April 8, 2024 11:00
@github-actions github-actions bot removed the 6.topic: stdenv Standard environment label Apr 8, 2024
@ashuramaruzxc ashuramaruzxc force-pushed the package/thcrap-steam-proton-wrapper branch from 03f2229 to 0becc15 Compare April 8, 2024 13:52
@msanft
Copy link
Contributor

msanft commented Apr 9, 2024

@ashuramaruzxc You'll want to reword format: use nixfmt-rfc-style instead of alejandra into thcrap-steam-proton-wrapper: init at 0-unstable-2024-04-03

@ashuramaruzxc ashuramaruzxc force-pushed the package/thcrap-steam-proton-wrapper branch from 084a73f to 134db91 Compare April 9, 2024 07:01
@ashuramaruzxc
Copy link
Member Author

ashuramaruzxc commented Apr 9, 2024

@ashuramaruzxc You'll want to reword format: use nixfmt-rfc-style instead of alejandra into thcrap-steam-proton-wrapper: init at 0-unstable-2024-04-03

Sorry, my bad, used commit message from the zsh history, noticed it right immediately, fixed...

@DontEatOreo
Copy link
Member

Adding yourself to the maintainers file needs to be a separate commit

@ashuramaruzxc ashuramaruzxc force-pushed the package/thcrap-steam-proton-wrapper branch from 134db91 to b0fbe66 Compare April 9, 2024 10:39
@DontEatOreo DontEatOreo requested a review from wegank April 11, 2024 07:57
@DontEatOreo DontEatOreo added the backport release-23.11 Backport PR automatically label Apr 11, 2024
@wegank wegank merged commit 2ceccc9 into NixOS:master Apr 11, 2024
26 of 27 checks passed
Copy link
Contributor

Successfully created backport PR for release-23.11:

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