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

Jump to bottom

Added Orchis-Theme and Reversal-Icon-Theme #102406

Closed
wants to merge 6 commits into from
Closed

Added Orchis-Theme and Reversal-Icon-Theme #102406

wants to merge 6 commits into from

Conversation

Icy-Thought
Copy link
Contributor

@Icy-Thought Icy-Thought commented Nov 1, 2020 •
edited

Motivation for this change

Adding both Orchis-Theme and Reversal-Icon-Theme packages to nixpkgs due to their absence in the nixpkgs repository.

Both default.nix files were written and shared by @ImExtends in the discord server as a solution to my request and after being granted the authorization to share the default.nix files, I chose to submit the packages to the main repository.

Due to me not being an experienced packager/maintainer, I would kindly request your help to help push forward the PR after confirming its working state.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.

SuperSandro2000
SuperSandro2000 requested changes Nov 2, 2020
View reviewed changes
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

You are missing the meta section.

SuperSandro2000
SuperSandro2000 requested changes Nov 2, 2020
View reviewed changes
pkgs/data/icons/reversal-icon-theme/default.nix Outdated Show resolved Hide resolved
pkgs/data/icons/reversal-icon-theme/default.nix Outdated
sha256 = "15vml083kd40hzfb6s1f5xndwqz6ys58sg076cvpz9vsbxhwbjnl";
};

buildInputs = [ gtk3 gnome-themes-extra gtk-engine-murrine bc ];
Copy link
Member

Choose a reason for hiding this comment

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

I looked at some other themes and they listed as gtk3 as native input. I think this could also be the case here. Also is bc required for the installer or at runtime?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checked the installation script and noticed that bc was not a used package. Not certain about it being used during runtime since the author did not mention anything regarding that matter in their README.md and thus I decided to exclude it from the default.nix file.

pkgs/data/themes/orchis-theme/default.nix Outdated Show resolved Hide resolved
@SuperSandro2000
Copy link
Member

Can you squash the commits together?

@Icy-Thought
Copy link
Contributor Author

Can you squash the commits together?

Was planning on doing so after my schedule had cleared up.

@Icy-Thought Icy-Thought closed this Nov 5, 2020
@Icy-Thought
Copy link
Contributor Author

Reason for closing: cannot seem to get git rebase to work as intended and have no time to resolve this issue.
If you want to re-submit the work, feel free to do so. Good luck!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 requested changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 0 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Icy-Thought @SuperSandro2000 @Janik-Haag