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

polybar: init at 3.0.4 #22222

Merged
merged 2 commits into from Feb 17, 2017
Merged

polybar: init at 3.0.4 #22222

merged 2 commits into from Feb 17, 2017

Conversation

afldcr
Copy link
Contributor

@afldcr afldcr commented Jan 28, 2017 •
edited

Polybar is a nice X11 status bar in the vein of i3bar or xmobar.

https://github.com/jaagr/polybar

Ignore the source branch name-- this developer moves fairly quickly :)

Reviewed points
  • package path fits guidelines
  • package name fits guidelines
  • package version fits guidelines
  • package build on ARCHITECTURE
  • executables tested on ARCHITECTURE
  • meta.description is set and fits guidelines
  • meta.license fits upstream license
  • meta.platforms is set
  • meta.maintainers is set
  • build time only dependencies are declared in nativeBuildInputs
  • source is fetched using the appropriate function
  • phases are respected
  • patches that are remotely available are fetched with fetchpatch
Possible improvements
  • Is there an easier way to maintain optional dependencies?
  • Is there any way to read the config.environment setting on NixOS?
Comments

@afldcr afldcr changed the title Initial commit adding the `polybar' package polybar: init at 3.0.3 Jan 28, 2017
@afldcr afldcr changed the title polybar: init at 3.0.3 polybar: init at 3.0.4 Feb 5, 2017
@afldcr
polybar: init at 3.0.4
0f905b0 
Polybar is a status bar in the vein of xmobar or i3bar.
@afldcr
Copy link
Contributor Author

afldcr commented Feb 5, 2017

Fairly sure I'm read to have this merged in, provided tests start passing.

asymmetric
asymmetric requested changes Feb 13, 2017
View reviewed changes
Copy link
Contributor

@asymmetric asymmetric left a comment

Choose a reason for hiding this comment

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

Small issues, otherwise works great! 👍

pkgs/applications/misc/polybar/default.nix
stdenv.mkDerivation rec {
name = "polybar-${version}";
version = "3.0.4";
src = fetchgit {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should use fetchFromGithub here, which downloads less data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sadly fetchFromGitHub doesn't include submodules, which causes the build process to break.

pkgs/applications/misc/polybar/default.nix Outdated
url = "https://github.com/jaagr/polybar";
rev = "1f31870d43f5cd87a5529a55b1d2d3e64105e0af";
sha256 = "1nhj4npqhs6zy161931sbdi52gz6163lik0wri9wr122sjf90jas";
fetchSubmodules = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

asymmetric
asymmetric reviewed Feb 13, 2017
View reviewed changes
pkgs/applications/misc/polybar/default.nix
(if i3GapsSupport then i3-gaps else null)
];

nativeBuildInputs = [
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should simply be buildInputs. nativeBuildInputs is used when cross-compiling, which isn't the case here.

EDIT: it actually makes sense to leave it as is, please disregard.

@globin globin merged commit b3c58dc into NixOS:master Feb 17, 2017
@afldcr afldcr mentioned this pull request Feb 23, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asymmetric asymmetric asymmetric requested changes

Assignees
No one assigned
Labels
2.status: work-in-progress 8.has: package (new) 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@afldcr @asymmetric @pSub @kevincox @globin @Janik-Haag