-
-
Notifications
You must be signed in to change notification settings - Fork 15.5k
gruvbox-dark-gtk: init at 1.0.1, gruvbox-dark-icons-gtk: init at 1.0.0 #109585
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
Conversation
I just renamed the title to more easily reflect the commits that are in here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please squash the package related commits together.
Well, that didn't work like intended... |
The right command is head |
(FWIW, You first do the git rebase command you used. Then, you choose the commits you want to squash, but you must order them to come DIRECTLY AFTER the commit you want to squash them into. e.g.
would become
I usually use |
Actually this is what I do all the time, too but it is still called squashing. |
That's exactly what I did, so I don't know what I did wrong.
Are you referring to this pull request 971f2a1? If yes, I don't know how to avoid doing that. After the rebase git wanted me to pull changes for some reason, and since I had local unpushed commits, the merge emerged. Is there a simple way to rescue this disaster? I'm thinking of attempting something like this, but figured I should probably ask someone who knows what to do before I screw things up more. Another thing I could try is to start from scratch and redo the changes in the PR, but I'd like to learn how to do it properly too. I learn from my mistakes ;) |
You can take the changes from this PR, delete the branch, create it under the new name and commit the changes into 3 commits and then force push. |
Instead of creating a merge commit, you usually want to @ofborg eval |
I was able to fix the problem by rebasing a second time, and using |
I have now added @romildo's suggested changes, some even in both files (like removing unneeded files, and not copying |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kdeFrameworks
doesn't appear in nix repl .
, which is probably why eval failed. I changed kdeFrameworks
to plasma5Packages
, reordered the commits, and force-pushed, which fixed the eval error.
Now, diff LGTM. GitHub ID also checks out. Thanks :)
Motivation for this change
I use these packages locally, and wanted them to become official. I have got them working properly, but the icon preview in lxappearance does not work. I don't know if it's a problem with the packaging, or the project. Icons seem to work nonetheless.
Please bear in mind that I'm new to NixOS, and contributing generally, so I might have missed something
Here are the git repositories for the projects:
https://github.com/jmattheis/gruvbox-dark-gtk
https://github.com/jmattheis/gruvbox-dark-icons-gtk
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
(completed withNo diff detected, stopping review...
)./result/bin/
) (The packages successfully build, and the themes can be applied using lxappearance)nix path-info -S
before and after)