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

Jump to bottom

weechatScripts.weechat-matrix: init at unstable-2019-11-10 #73460

Merged
merged 4 commits into from Nov 17, 2019
Merged

weechatScripts.weechat-matrix: init at unstable-2019-11-10 #73460

merged 4 commits into from Nov 17, 2019

Conversation

tilpner
Copy link
Member

@tilpner tilpner commented Nov 15, 2019 •
edited

Motivation for this change

This migrates the package from my NUR package set, as multiple people have attempted to repackage it due to poor visibility of NUR packages.

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 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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

@clefru, @zfnmxt, @redvers, @gjabell: Please test this

@tilpner
Copy link
Member Author

tilpner commented Nov 15, 2019

@GrahamcOfBorg build weechatScripts.weechat-matrix

gjabell
gjabell suggested changes Nov 15, 2019
View reviewed changes
Copy link
Contributor

@gjabell gjabell left a comment

Choose a reason for hiding this comment

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

Works for me! Would it be possible to wrap the contrib/matrix_upload (see here) script?

@tilpner
Copy link
Member Author

tilpner commented Nov 15, 2019

@gjabell I've added the matrix_upload script, and uploading seems to work if weechat-matrix is installed so that weechat has it in PATH.

@clefru
Copy link
Contributor

clefru commented Nov 15, 2019

@tilpner Works for me! Thank you so much for putting this together :)

@zfnmxt
Copy link
Contributor

zfnmxt commented Nov 16, 2019

Works great for me. Thanks a lot. Surprised you even remember me asking about this on #nixos. :)

jonringer
jonringer reviewed Nov 16, 2019
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

pkgs/development/python-modules/matrix-nio/default.nix Show resolved Hide resolved
jonringer
jonringer reviewed Nov 16, 2019
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

just kidding :), found some

pkgs/applications/networking/irc/weechat/scripts/weechat-matrix/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/irc/weechat/scripts/weechat-matrix/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/irc/weechat/scripts/weechat-matrix/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/irc/weechat/scripts/weechat-matrix/default.nix Outdated Show resolved Hide resolved
@dtzWill dtzWill mentioned this pull request Nov 16, 2019
Closed
10 tasks
@tilpner
Copy link
Member Author

tilpner commented Nov 16, 2019

@jonringer Thank you, I pushed changes to address your comments.

@tilpner tilpner mentioned this pull request Nov 16, 2019
Merged
@jonringer
Copy link
Contributor

@GrahamcOfBorg build weechatScripts.weechat-matrix

jonringer
jonringer approved these changes Nov 17, 2019
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

nix-reveiw passes on NixOS (1 python38 failure, but that's an upstream issue)
diff LGTM

[1 built (1 failed), 7 copied (49.8 MiB), 10.0 MiB DL]
error: build of '/nix/store/pfs6cdd27064rrldxvl0s982jwb84p6m-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/73460
1 package failed to build:
python38Packages.matrix-nio

9 package were build:
matrique mtxclient nheko olm purple-matrix python27Packages.python-olm python37Packages.matrix-nio python37Packages.python-olm python38Packages.python-olm

pkgs/applications/networking/irc/weechat/scripts/default.nix
@@ -1,10 +1,12 @@
{ callPackage, luaPackages }:
{ callPackage, luaPackages, python3Packages }:
Copy link
Contributor

Choose a reason for hiding this comment

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

this should probably be python and in all-packages this should be passed python = python3; then use python.pkgs.callPackage; but I don't feel strongly enough about that to require a change.

@jonringer jonringer merged commit 8150b58 into NixOS:master Nov 17, 2019
@tilpner tilpner deleted the pkgs/weechat-matrix/init branch November 17, 2019 08:48
@tilpner tilpner restored the pkgs/weechat-matrix/init branch November 17, 2019 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gjabell gjabell gjabell requested changes

@jonringer jonringer jonringer approved these changes

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tilpner @clefru @zfnmxt @jonringer @gjabell