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

sclack: init at unstable-2019-07-15 #72994

Closed
wants to merge 5 commits into from
Closed

sclack: init at unstable-2019-07-15 #72994

wants to merge 5 commits into from
+116 −2

Conversation

evanjs
Copy link
Member

@evanjs evanjs commented Nov 7, 2019 •
edited
Loading

Motivation for this change

#59900 -- add sclack package
Stumbled across this issue and hadn't heard of sclack before.
Works pretty well, especially compared to e.g. slack-term.

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.

TODO

  • config.json
    • I'm just copying over the default config.json, is this enough? Or should this have a module with extraConfig, etc? Maybe configuring this option could be deferred to home-manager, etc.
  • Mac Support
    • While urwid lists support for linux and darwin, urwid_readline only lists support for linux. Can a darwin user confirm whether this works on mac?
  • app.py
    • Is there a cleaner way to install the executable (vs moving app.py to sclack)?
  • asyncio
    • @exarkun recommended filing a bug upstream since asyncio should be a conditional dependency. Which is still odd to me as the repo requires 3.4+ anyway... Should I block on that and leave my workaround in if the author declines? Or is there a better way to handle it here, anyway?
  • emoji
    • emoji don't seem to work right away, do we need to do something with fontconfig, or at least provide a font option, etc? This seems to be the case for only a few emoji, e.g. 🤷‍♂️, most emoji seem to work fine.
    • sclack - Nerd Fonts

I think that's it, but I'm happy to address anything else I missed.

Sorry, something went wrong.

@ofborg ofborg bot added 6.topic: python 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: 1-10 10.rebuild-linux: 1-10 labels Nov 7, 2019
@ryantm ryantm added the 2.status: work-in-progress This PR isn't done label Jan 6, 2020
@stale
Copy link

stale bot commented Jun 1, 2020

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the
    related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
    irc.freenode.net.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 1, 2020
@evanjs
Copy link
Member Author

evanjs commented Jun 3, 2020

Curious what others think, but I could add the config option to the accepted arguments, that might be enough to get this on linux, at least.

If that’s enough to get this merged, then this shouldn’t be too much work.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 3, 2020
@sikmir
Copy link
Member

sikmir commented Aug 3, 2020

@evanjs Please rebase to resolve conflict. I would like to test it on darwin.

evanjs and others added 3 commits August 3, 2020 07:54
@evanjs
pythonPackages.urwid_readline: init at 0.10

Verified

This commit was signed with the committer’s verified signature.
vcunat Vladimír Čunát
GPG key ID: E747DF1F9575A3AA
Verified
Learn about vigilant mode
5ee02b3
@evanjs
pythonPackages.sclack: init at unstable-2019-07-15
37ea52f
@evanjs
sclack: fix some changes introduced by slackclient 2.0
c7578de 
pythonPackages.urwid_readline: 0.10 -> 0.11
pythonPackages.urwid: fix build/tests
@evanjs
Copy link
Member Author

evanjs commented Aug 7, 2020

@sikmir I have pushed a couple additional changes on top of the rebase

Unfortunately, there seem to have been some breaking changes in slackclient, and as sclack has not been updated in some time, it looks like there is some work that is needed to get it working with the updated slackclient api (2.0+).

Here is what my changes produce when the program is configured with a valid xoxs- token (which I got with the method mentioned here but there are multiple methods:

Whoops, something went wrong:
api_call() got an unexpected keyword argument 'exclude_archived'
File "/nix/store/d5pgn8jwdkjl23mx79wddkgx7a2hjijp-python3.8-sclack-unstable-2019-07-15/bin/.sclack-wrapped", line 147, in component_did_mount
yield from self.mount_sidebar(executor)
File "/nix/store/d5pgn8jwdkjl23mx79wddkgx7a2hjijp-python3.8-sclack-unstable-2019-07-15/bin/.sclack-wrapped", line 152, in mount_sidebar
yield from asyncio.gather(
File "/nix/store/8hf2rpqx8fnx6a3jm9pvn0d40c065zxl-python3-3.8.3/lib/python3.8/concurrent/futures/thread.py", line 57, in run
result = self.fn(*self.args, **self.kwargs)
File "/nix/store/d5pgn8jwdkjl23mx79wddkgx7a2hjijp-python3.8-sclack-unstable-2019-07-15/lib/python3.8/site-packages/sclack/store.py", line 149, in load_channels
conversations = self.slack.api_call(

There is still an error around store.py#L149

@ryantm ryantm added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 3, 2020
@stale
Copy link

stale bot commented Jun 7, 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 Jun 7, 2021
@evanjs
Copy link
Member Author

evanjs commented Jun 21, 2022

Not using this at the moment and there is some work on the project (last updated 2021/05/20 via haskellcamargo/sclack@da75272) needed to get it working with the updated slackclient api (2.0+).

@evanjs evanjs closed this Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: work-in-progress This PR isn't done 6.topic: python 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 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
3 participants
@evanjs @sikmir @ryantm