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

sdl3-ttf: init at 3.2.0, maintainers: add charain #390310

Merged
merged 2 commits into from
Mar 18, 2025

Conversation

chai-yuan
Copy link
Contributor

@chai-yuan chai-yuan commented Mar 16, 2025

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/)
  • 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.

@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Mar 16, 2025
Copy link
Contributor

@LordGrimmauld LordGrimmauld left a comment

Choose a reason for hiding this comment

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

A couple notes:
Commits: this PR should be 2 commits. One adding you to the maintainer list, one adding sdl3-ttf. They should be titled maintainers: add charain and sdl3-ttf: init at 3.2.0

You are using github as a source. This is completely fine. It then might make sense to do nix-update-script too, so the bot can automatically update this package.

You are using a enableTests override. Previous SDL and SDL_ttf packages had such an override mostly because darwin kept breaking. If the tests pass on darwin, there is no need for that override field. If they are breaking on darwin, then set a default accordingly.

cmake: you are using cmake with gnu autotools backend. Might make sense to add ninja to nativeBuildInputs.

meta: maybe consider adding pkgConfigModules field and add pkg-config validation to passthru?

buildInputs: the darwin additional inputs look a bit funny, did you test this?

@chai-yuan chai-yuan changed the title Add sdl3 ttf sdl3-ttf: init at 3.2.0 maintainers: add charain Mar 16, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin and removed 10.rebuild-darwin: 1 10.rebuild-darwin: 1-10 labels Mar 16, 2025
@chai-yuan chai-yuan marked this pull request as draft March 16, 2025 10:30
@chai-yuan chai-yuan force-pushed the add-sdl3-ttf branch 2 times, most recently from 9aa7480 to d5a42e4 Compare March 16, 2025 10:56
@github-actions github-actions bot added 10.rebuild-darwin: 1 10.rebuild-darwin: 1-10 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin labels Mar 16, 2025
@LordGrimmauld
Copy link
Contributor

At a glance this is looking significantly better already 👍

Copy link
Contributor

@LordGrimmauld LordGrimmauld left a comment

Choose a reason for hiding this comment

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

You seem to have hash mismatches. Also, hash is preferred over sha256.

@chai-yuan chai-yuan force-pushed the add-sdl3-ttf branch 2 times, most recently from fc018d8 to 0506a8e Compare March 16, 2025 12:43
@Anillc Anillc assigned Emin017 and unassigned Emin017 Mar 16, 2025
@Anillc Anillc requested a review from Emin017 March 16, 2025 12:46
@chai-yuan chai-yuan marked this pull request as ready for review March 16, 2025 12:48
@Emin017
Copy link
Member

Emin017 commented Mar 16, 2025

Successfully build this on aarch64-darwin.

@Emin017
Copy link
Member

Emin017 commented Mar 16, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 390310


aarch64-darwin

✅ 1 package built:
  • sdl3-ttf

@LordGrimmauld
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 390310


x86_64-linux

✅ 1 package built:
  • sdl3-ttf

Copy link
Contributor

@LordGrimmauld LordGrimmauld left a comment

Choose a reason for hiding this comment

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

Seems to build on both aarch64-linux and x86_64-linux. Set the platforms as marcin requested, and maybe wait for someone with a darwin machine to build this. Other than that, this seems about ready.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Mar 16, 2025
@Emin017
Copy link
Member

Emin017 commented Mar 17, 2025

Seems to build on both aarch64-linux and x86_64-linux. Set the platforms as marcin requested, and maybe wait for someone with a darwin machine to build this. Other than that, this seems about ready.

I tested build on aarch64-darwin, it works well.

@wegank wegank added 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people and removed 12.approvals: 1 This PR was reviewed and approved by one reputable person labels Mar 17, 2025
@LordGrimmauld LordGrimmauld requested a review from getchoo March 17, 2025 23:01
@FliegendeWurst FliegendeWurst added the 8.has: package (new) This PR adds a new package label Mar 18, 2025
@FliegendeWurst FliegendeWurst changed the title sdl3-ttf: init at 3.2.0 maintainers: add charain sdl3-ttf: init at 3.2.0, maintainers: add charain Mar 18, 2025
Co-authored-by: Qiming Chu <cchuqiming@gmail.com>
Co-authored-by: Anillc <i@anillc.cn>
Copy link
Member

@FliegendeWurst FliegendeWurst left a comment

Choose a reason for hiding this comment

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

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 390310


x86_64-linux

✅ 1 package built:
  • sdl3-ttf

@FliegendeWurst FliegendeWurst merged commit 48eb2dc into NixOS:master Mar 18, 2025
28 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people 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.

8 participants