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

tmuxPlugins.dracula: init at unstable-2020-12-2 #104652

Merged
merged 2 commits into from Dec 2, 2020
Merged

tmuxPlugins.dracula: init at unstable-2020-12-2 #104652

merged 2 commits into from Dec 2, 2020

Conversation

ethancedwards8
Copy link
Member

Motivation for this change

Adding the tmux plugin Dracula, it is a theme among other things like cpu usage, ram usage, and battery percentage.

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.

@ethancedwards8
Copy link
Member Author

Hmmm I don't think everything should be rebuilding like this?

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 104652 run on x86_64-linux 1

1 package built:
  • tmuxPlugins.dracula

@SuperSandro2000
Copy link
Member

Hmmm I don't think everything should be rebuilding like this?

For me there is just one rebuild which is expected when adding a new plugin. What do you think is going wrong here exactly?

@ethancedwards8
Copy link
Member Author

I'm actually not sure then, it just looked like a lot of packages were building when I ran my tests locally. I guess it all checks out. @SuperSandro2000

buckley310
buckley310 reviewed Nov 23, 2020
View reviewed changes
pkgs/misc/tmux-plugins/default.nix Outdated
Comment on lines 138 to 139
};
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation here is slightly off

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed. 👍

@SuperSandro2000
Copy link
Member

I'm actually not sure then, it just looked like a lot of packages were building when I ran my tests locally. I guess it all checks out. @SuperSandro2000

They probably where build because you didn't have them locally and they where not available from cache. What counts is what needs to be built before and after the PR.

@ethancedwards8
Copy link
Member Author

I'm actually not sure then, it just looked like a lot of packages were building when I ran my tests locally. I guess it all checks out. @SuperSandro2000

They probably where build because you didn't have them locally and they where not available from cache. What counts is what needs to be built before and after the PR.

Okay makes sense.

@ethancedwards8
Copy link
Member Author

Also, this is my first nixpkgs PR so if I'm doing anything else wrong let me know and I'll be sure to fix it.

@buckley310
Copy link
Contributor

Squashing all the commits into a single commit is generally preferred.
Also for consistency you probably ought to rename the PR to be the same as the commit message.

@SuperSandro2000 SuperSandro2000 changed the title Tmux plugin, dracula. tmuxPlugins.dracula: init at 1.0.1 Nov 25, 2020
@ethancedwards8 ethancedwards8 force-pushed the tmux-plugins-dracula branch 2 times, most recently from 72fde59 to 3a6ce09 Compare November 26, 2020 17:24
@ethancedwards8
Copy link
Member Author

Ahh, I've never squashed anything before, and I'm failing miserably at it.

@ethancedwards8 ethancedwards8 force-pushed the tmux-plugins-dracula branch 3 times, most recently from e83c29e to 2f03fb2 Compare November 27, 2020 05:14
@ethancedwards8
Copy link
Member Author

And now I rebase...

@ethancedwards8 ethancedwards8 force-pushed the tmux-plugins-dracula branch 3 times, most recently from 75032ac to 548e3b8 Compare November 29, 2020 08:25
@ethancedwards8
Copy link
Member Author

Thanks for the help, this has definitely taught me a lot about git and Nix.

timstott
timstott approved these changes Nov 30, 2020
View reviewed changes
Copy link
Contributor

@timstott timstott left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution @ethancedwards8, this is excellent 🎩

@timstott timstott mentioned this pull request Dec 1, 2020
Closed
10 tasks
@ethancedwards8
Copy link
Member Author

ethancedwards8 commented Dec 1, 2020 •
edited

Ah, didn't notice there was already a PR, whoops. Since I'm a maintainer of the actual plug-in I feel it's right I package it :)

SuperSandro2000
SuperSandro2000 reviewed Dec 1, 2020
View reviewed changes
pkgs/misc/tmux-plugins/default.nix Outdated
src = fetchFromGitHub {
owner = "dracula";
repo = "tmux";
rev = "cb1d420a6267c600a8ef0e7a3c00e2474b30ae99";
Copy link
Member

Choose a reason for hiding this comment

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

But this will. Please either tag the releases in your repo or switch the version to unstable-year-month-date.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. 👍

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 104652 run on x86_64-darwin 1

1 package built:
  • tmuxPlugins.dracula

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 104652 run on x86_64-linux 1

1 package built:
  • tmuxPlugins.dracula

@ethancedwards8 ethancedwards8 changed the title tmuxPlugins.dracula: init at 1.0.1 tmuxPlugins.dracula: init at unstable-2020-12-2 Dec 2, 2020
@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 104652 run on x86_64-darwin 1

1 package built:
  • tmuxPlugins.dracula

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 104652 run on x86_64-linux 1

1 package built:
  • tmuxPlugins.dracula

@SuperSandro2000 SuperSandro2000 merged commit 61e6343 into NixOS:master Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@buckley310 buckley310 buckley310 left review comments

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@timstott timstott timstott approved these changes

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ethancedwards8 @SuperSandro2000 @buckley310 @timstott @Janik-Haag