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

regolith: init at 1.5.1 #374961

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

regolith: init at 1.5.1 #374961

wants to merge 2 commits into from
+49 −0

Conversation

arexon
Copy link

@arexon arexon commented Jan 19, 2025

Regolith is a tool that helps with creation of Resource Pack & Behavior Pack for Minecraft Bedrock Edition.

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.

@github-actions github-actions bot added the 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` label Jan 19, 2025
@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jan 19, 2025
@Adda0
Copy link
Contributor

Adda0 commented Jan 19, 2025 •
edited
Loading

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 374961

x86_64-linux

✅ 1 package built:
  • regolith

Reviewed points

✅ 26 | 🔴 0
  • All package paths fit the guidelines (e.g., pkgs/by-name/so/some-package/package.nix for top-level packages).
  • Package name fits guidelines.
  • Package version fits guidelines.
  • The packages sources are fetched using the appropriate functions (e.g., packages from GitHub should use fetchFromGitHub).
  • The package sources use mirror:// URLs when available.
  • tag is used instead of rev for tag versions.
  • hash is used instead of sha256 for hashes.
  • Build time only dependencies are declared in nativeBuildInputs or its equivalent in the used builder.
  • The list of phases is not overridden.
  • When a phase (like installPhase) is overridden, it starts with runHook preInstall and ends with runHook postInstall.
  • Patches have a comment describing either the upstream URL or a reason why the patch wasn't upstreamed.
  • Patches that are remotely available are fetched rather than vendored.
  • Package builds on x86_64-linux.
  • Executables tested on x86_64-linux.
  • All the meta fields fit the guidelines and contain the correct information:
    • meta.description is set and fits guidelines.
      • Descriptions should not contain complete sentences and should consist of phrases that fill the blank in " is a/an/the ____" (starting with a capital letter).
    • meta.license fits upstream license.
    • meta.platforms should be set (or the package will not get binary substitutes).
    • Maintainers in meta.maintainers must be set.
      • This can be the package submitter or a community member that accepts taking up maintainership of the package.
    • The meta.mainProgram must be set if a main executable exists.
  • Whatever is needed is brought into scope instead of using top-level pkgs to make it easier to override.
  • General with lib; is not used.
  • If possible, update script is provided.
    • DONE.
  • If possible, tests are provided.
    • RESOLVED.
  • The code contains no typos and all typos have been reported.

Possible improvements

Comments

Hey. Thank you for the contribution. Looks good. Just some minor details.

Adda0
Adda0 suggested changes Jan 19, 2025
View reviewed changes
pkgs/by-name/re/regolith/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/re/regolith/package.nix Show resolved Hide resolved
pkgs/by-name/re/regolith/package.nix Show resolved Hide resolved
@Adda0 Adda0 added the 8.has: package (new) This PR adds a new package label Jan 19, 2025
@Adda0
Copy link
Contributor

Adda0 commented Jan 19, 2025

Now just squash all commits regarding the package into a single ... init at... commit, and it looks good to me.

@arexon arexon requested a review from Adda0 January 19, 2025 11:54
Adda0
Adda0 approved these changes Jan 19, 2025
View reviewed changes
Copy link
Contributor

@Adda0 Adda0 left a comment

Choose a reason for hiding this comment

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

Looks great. Builds and runs. Cheers.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 19, 2025
@arexon
regolith: init at 1.5.1
e48e51e 
regolith: use string literal in repo attribute

regolith: add update script

regolith: add comment explaining why tests are disabled

regolith: add versionCheckHook, --version arg checking, and enable install check
@Defelo
Copy link
Member

Defelo commented Jan 19, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 374961

x86_64-linux

✅ 1 package built:
  • regolith

aarch64-linux

✅ 1 package built:
  • regolith

@Defelo Defelo added 12.approvals: 2 This PR was reviewed and approved by two reputable people and removed 12.approvals: 1 This PR was reviewed and approved by one reputable person labels Jan 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Adda0 Adda0 Adda0 approved these changes

@Defelo Defelo Defelo approved these changes

Assignees
No one assigned
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-linux: 1-10 12.approvals: 2 This PR was reviewed and approved by two reputable people 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@arexon @Adda0 @Defelo @wegank @NixOSInfra