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

cheat: init at 2.1.26 #18554

Merged
merged 1 commit into from
Sep 16, 2016
Merged

cheat: init at 2.1.26 #18554

merged 1 commit into from
Sep 16, 2016

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Sep 13, 2016

Motivation for this change
Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • OS X
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@Mic92, thanks for your PR! By analyzing the annotation information on this pull request, we identified @FRidh to be a potential reviewer

@FRidh
Copy link
Member

FRidh commented Sep 13, 2016

@Mic92 judging from the description of this package it seems it is an application, and not a Python library. Or is it both? If it is used as an application only, it needs to be moved outside of python-packages.nix.

@FRidh FRidh added 6.topic: python 8.has: package (new) This PR adds a new package labels Sep 13, 2016

propagatedBuildInputs = with self; [ docopt pygments ];

src = pkgs.fetchFromGitHub {
Copy link
Member

Choose a reason for hiding this comment

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

the package seems to be available on PyPI. For Python packages PyPI is the preferred source.

@Mic92 Mic92 force-pushed the cheat branch 2 times, most recently from a50ec86 to 8505df5 Compare September 13, 2016 15:59
@Mic92
Copy link
Member Author

Mic92 commented Sep 13, 2016

fixed all concerns

@@ -6281,6 +6281,8 @@ in

cgdb = callPackage ../development/tools/misc/cgdb { };

cheat = callPackage ../data/documentation/cheat { };
Copy link
Member

Choose a reason for hiding this comment

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

data category is for data (not applications). A better place for cheat is applications/office or applications/misc (IMHO)

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved it to applications/misc now, it does definitely not belong to application/office. Its core asset are actually these short text files containing documentation like manpages or gnome-user-docs.

Copy link
Member

Choose a reason for hiding this comment

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

application/misc is fine for me. Though, I should note that application/office doesn't mean application should deal with large nicely formatted documents and this kind of things.

In example, we currently have todo.txt-cli and tudu in application/office, which both are small apps for managing todo lists. There are also keepnote and eventlist, which are GUI apps for organizing other things.

Copy link
Member Author

Choose a reason for hiding this comment

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

cheat is not a todo/note manager.

Copy link
Member

Choose a reason for hiding this comment

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

IMHO, it's basically a note management software.

@rasendubi
Copy link
Member

LGTM

@Mic92
Copy link
Member Author

Mic92 commented Sep 15, 2016

anything left to do?

@rasendubi rasendubi merged commit 917bb97 into NixOS:master Sep 16, 2016
@Mic92 Mic92 deleted the cheat branch October 2, 2016 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: python 8.has: package (new) This PR adds a new package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants