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

weechat-autojoin: init at 0.3.1; weechat-text-item: init at 0.9 #66786

Closed
wants to merge 4 commits into from
Closed

weechat-autojoin: init at 0.3.1; weechat-text-item: init at 0.9 #66786

wants to merge 4 commits into from

Conversation

bdesham
Copy link
Contributor

@bdesham bdesham commented Aug 17, 2019 •
edited

Motivation for this change

Adds two scripts to be used with the Weechat chat client:

  • weechat-autojoin: When quitting Weechat, automatically saves the list of current IRC channels so that they can be joined next time Weechat is launched.
  • weechat-text-item: Allows pieces of static text to be included in Weechat’s various status bars.

These scripts come from the Weechat scripts site, which doesn’t seem to offer a way to download a specific version of a script—only the latest version. Therefore, I included the two scripts directly within Nixpkgs. They’re 6,703 bytes and 10,764 bytes. If anyone knows of a way to get specific versions from the Weechat site that would obviously be preferable. (The text_item script says that development is hosted at https://github.com/weechatter/weechat-scripts, but the Weechat site has a more recent version of the script than that repo does.) This problem is now fixed; the derivations just use fetchFromGitHub like you’d expect.

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

cc @lovek323 @the-kenny @lheckemann @Ma27 (the maintainers of Weechat itself)

@bdesham bdesham changed the title Weechat scripts weechat-autojoin: init at 0.3.0; weechat-text-item: init at 0.9 Aug 17, 2019
@lheckemann
Copy link
Member

This should probably download the scripts using fetchurl rather than adding them verbatim into nixpkgs.

@bdesham
Copy link
Contributor Author

bdesham commented Aug 20, 2019

I thought that downloading specific versions of these scripts was impossible—that you could only download the latest versions—but I was mistaken. I’ll update the PR tonight, hopefully.

@jtojnar
Copy link
Contributor

jtojnar commented Aug 20, 2019

The text_item looks the same from both sources here.

@bdesham
Copy link
Contributor Author

bdesham commented Aug 20, 2019

They weren’t the same when I opened this PR 😉 It turns out that all of the scripts on the WeeChat scripts site are stored in https://github.com/weechat/scripts, so I’m going to update these derivations to pull from there.

@bdesham
Copy link
Contributor Author

bdesham commented Aug 21, 2019

This has been updated to use fetchFromGitHub instead of including the actual scripts in Nixpkgs.

@jtojnar
Copy link
Contributor

jtojnar commented Aug 21, 2019

Could you perhaps factor out the common code to a common.nix file?

@bdesham
Copy link
Contributor Author

bdesham commented Aug 25, 2019

@jtojnar I created a mk-weechat-script.nix file that contains a function with all of the common code. If this seems like a good approach then I will add some documentation of how the function can be used.

The way I’m injecting stdenv and fetchFromGitHub into the function feels extremely awkward right now. I’d appreciate any advice on how to make that more natural.

@jtojnar
Copy link
Contributor

jtojnar commented Aug 26, 2019

Why not just use callPackage on it?

@bdesham bdesham force-pushed the weechat-scripts branch 2 times, most recently from deeb9d5 to 2346aac Compare September 2, 2019 17:47
@bdesham
Copy link
Contributor Author

bdesham commented Sep 2, 2019

@jtojnar Thanks for the pointers on IRC. I’ve updated this to be a little simpler.

@bdesham
Copy link
Contributor Author

bdesham commented Jan 16, 2020

I’ve rebased this against the latest Nixpkgs master and I updated one of the scripts.

@bdesham bdesham changed the title weechat-autojoin: init at 0.3.0; weechat-text-item: init at 0.9 weechat-autojoin: init at 0.3.1; weechat-text-item: init at 0.9 Jan 26, 2020
@Ma27 Ma27 removed their request for review May 29, 2020 08:35
@stale
Copy link

stale bot commented Nov 26, 2020

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 26, 2020
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 18, 2021
@stale
Copy link

stale bot commented Jul 19, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 19, 2021
@bdesham
Copy link
Contributor Author

bdesham commented Sep 30, 2022

I still think this is a useful change, but I don’t have the energy to shepherd it through the nixpkgs PR process so I’m going to close it.

@bdesham bdesham closed this Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lovek323 lovek323 Awaiting requested review from lovek323

@the-kenny the-kenny Awaiting requested review from the-kenny

@lheckemann lheckemann Awaiting requested review from lheckemann

Assignees
No one assigned
Labels
2.status: merge conflict 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@bdesham @lheckemann @jtojnar @SuperSandro2000