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

chatterino2: init at unstable-2019-05-11 #60747

Merged
merged 2 commits into from
May 11, 2019
Merged

chatterino2: init at unstable-2019-05-11 #60747

merged 2 commits into from
May 11, 2019
+35 −0

Conversation

rexim
Copy link
Contributor

@rexim rexim commented May 2, 2019

Motivation for this change

Chatterino is a chat client for Twitch chat. It aims to be an improved/extended version of the Twitch web chat. It's actively used in the Twitch community.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

cc @fourtf @pajlada

Sorry, something went wrong.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels May 2, 2019
@artemist
Copy link
Member

artemist commented May 2, 2019

This package otherwise looks good. Thanks for changing that so quickly!

aanderse
aanderse requested changes May 6, 2019
View reviewed changes
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

Thanks for choosing to contribute to nixpkgs! I have included a few suggestions which I hope will be helpful.

After you're ready with this PR you should rewrite the git history to be 2 commits, the first of which is you adding yourself to the maintainer list and the second adding the package (ie. everything else squashed).

Please let me know if you have any questions.

@pajlada
Copy link

pajlada commented May 7, 2019

is "irc" the appropriate category for this package?
The technology that drives the client is IRC, yes, but it's a chat client that can right now only join twitch.tv chats

@rexim
Copy link
Contributor Author

rexim commented May 7, 2019

@pajlada yeah, I agree. Maybe instant-messengers is a better category?

@rexim
Copy link
Contributor Author

rexim commented May 7, 2019

@aanderse thank you for the remarks! No questions so far. My NixOS machine recently broke, I'll address the remarks as soon as I get it back working. Within a week or so.

@pajlada
Copy link

pajlada commented May 7, 2019

@rexim I'm not the most familiar with the categories of packages in NixOS, but from what I can see chatterino fits better under instant-messengers yeah

@rexim
Add rexim to maintainers

Verified

This commit was signed with the committer’s verified signature.
sethmlarson Seth Michael Larson
GPG key ID: 7D5EC9FE0DE2C795
Verified
Learn about vigilant mode
ccfe8ec
@rexim
Copy link
Contributor Author

rexim commented May 10, 2019

@aanderse I hope I addressed most of the concerns. However there is one more thing. I recenly learned that the version in tag 2.0.4 is very old and the most relevant changes are in the nightly branch. So the application has kind of a rolling release nature. I changed the version to 2.0.4-nightly and rev to the latest in nightly and will periodically bump it when enough changes in nightly are accumulated. Is that an appropriate way of maintaining packages in nixpkgs?

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin labels May 10, 2019
@aanderse
Copy link
Member

@rexim Awesome work! As for the version, not a problem at all, very common. From the nixpkgs manual:

If a package is not a release but a commit from a repository, then the version part of the name must be the date of that (fetched) commit. The date must be in "YYYY-MM-DD" format. Also append "unstable" to the name - e.g., "pkgname-unstable-2014-09-23".

@rexim rexim changed the title chatterino2: init at 2.0.4 chatterino2: init at unstable-2019-05-11 May 10, 2019
@rexim
Copy link
Contributor Author

rexim commented May 10, 2019

@aanderse thank you so much for the quick response! Updated the version accordingly.

@rexim @aanderse
chatterino2: init at unstable-2019-05-11

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
79586ca
@aanderse
Copy link
Member

@GrahamcOfBorg build chatterino2

@aanderse aanderse merged commit 39a1f3c into NixOS:master May 11, 2019
@aanderse
Copy link
Member

@rexim made 2 tiny changes before merging: consistent spacing in longDescription and removed redundant stdenv.lib text.

Thanks for your contribution to nixpkgs! 🎉

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

@pajlada pajlada

@artemist artemist

@symphorien symphorien

@aanderse aanderse

Assignees
No one assigned
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@rexim @artemist @pajlada @aanderse @symphorien