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

ps3-disc-dumper: 3.2.3 -> 4.2.5, .NET 6 -> 9 #361506

Merged
merged 7 commits into from
Dec 13, 2024

Conversation

gepbird
Copy link
Contributor

@gepbird gepbird commented Dec 3, 2024

Tracking: #326335

A big change since the previous version is the removal of the CLI project (13xforever/ps3-disc-dumper@a88facc). Now we are building the GUI project made in Avalonia.

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
  • Tested basic functionality of all binary files (usually in ./result/bin/) only checked whether the GUI opens
  • 25.05 Release Notes (or backporting 24.11 and 25.05 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.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog labels Dec 3, 2024
@gepbird gepbird requested a review from evanjs December 3, 2024 18:24
@gepbird gepbird mentioned this pull request Dec 3, 2024
17 tasks
@gepbird gepbird force-pushed the ps3-disc-dumper-4.2.5 branch from 06f0a8f to c3ea9e7 Compare December 3, 2024 18:36
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Dec 3, 2024
@gepbird gepbird force-pushed the ps3-disc-dumper-4.2.5 branch from c3ea9e7 to c61c752 Compare December 3, 2024 19:26
@ofborg ofborg bot added the 11.by: package-maintainer This PR was created by the maintainer of the package it changes label Dec 4, 2024
@gepbird gepbird force-pushed the ps3-disc-dumper-4.2.5 branch from 40311f6 to 3944510 Compare December 4, 2024 16:26
@gepbird
Copy link
Contributor Author

gepbird commented Dec 4, 2024

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 361506


x86_64-linux

⏩ 1 package blacklisted:
  • nixos-install-tools
✅ 1 package built:
  • ps3-disc-dumper

@GGG-KILLER
Copy link
Contributor

Also, please squash all of this into a single commit with the proper title

@gepbird gepbird force-pushed the ps3-disc-dumper-4.2.5 branch 3 times, most recently from 666c63e to 3e76df8 Compare December 10, 2024 18:37
@gepbird
Copy link
Contributor Author

gepbird commented Dec 10, 2024

Also, please squash all of this into a single commit with the proper title

I've edited the previous commits, usually I like keeping every small meaningful change in a separate commit. I'm open to squashing all the refactor commits into a single one if that's preferred in nixpkgs, but for good bisecting and reviewing experience I'd like to keep nixfmt, package update and general refactor changes in separate commits

@GGG-KILLER
Copy link
Contributor

I've edited the previous commits, usually I like keeping every small meaningful change in a separate commit. I'm open to squashing all the refactor commits into a single one if that's preferred in nixpkgs, but for good bisecting and reviewing experience I'd like to keep nixfmt, package update and general refactor changes in separate commits

I don't remember seeing something similar in other nixpkgs PRs so I'd err on the side of caution and request that they be squashed, however if you'd like to request a second opinion from another nixpkgs member or just ignore my nit I have no problems with it.

@gepbird
Copy link
Contributor Author

gepbird commented Dec 13, 2024

I prefer the current commit style, but again if there's a stronger opinion on how it should be changed let me know.

10 days have passed and @evanjs hasn't responded whether the replacement of the CLI project to a GUI one is acceptable (as decided by upstream where an issue was raised about this). If anyone reports the same in nixpkgs we can add the CLI project that is pinned to an older, non-maintained version. For now I think this PR is ready to merge.

cc @GGG-KILLER @corngood

@corngood
Copy link
Contributor

I personally prefer the smaller commits, as long as each one is meant work on its own.

@corngood
Copy link
Contributor

I'm thinking we shouldn't backport this to 24.11, even though the package is marked insecure there.

If we could get the old one building with a new SDK, maybe we could backport that part, but I'm guessing that wouldn't be easy.

@gepbird gepbird force-pushed the ps3-disc-dumper-4.2.5 branch from 3e76df8 to 47b8baa Compare December 13, 2024 15:33
@gepbird gepbird force-pushed the ps3-disc-dumper-4.2.5 branch from 47b8baa to c5caab6 Compare December 13, 2024 16:02
@gepbird
Copy link
Contributor Author

gepbird commented Dec 13, 2024

I'm thinking we shouldn't backport this to 24.11, even though the package is marked insecure there.

If we could get the old one building with a new SDK, maybe we could backport that part, but I'm guessing that wouldn't be easy.

I separated the .NET version bump and package version bump into 2 commits, both seems to work.
A manual backport of this PR excluding the last commit should be good.

@corngood
Copy link
Contributor

Nice one. Thanks

@corngood
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 361506


x86_64-linux

⏩ 1 package blacklisted:
  • nixos-install-tools
✅ 1 package built:
  • ps3-disc-dumper

@corngood corngood merged commit 474104e into NixOS:master Dec 13, 2024
24 of 25 checks passed
@gepbird gepbird deleted the ps3-disc-dumper-4.2.5 branch December 13, 2024 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants