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

Add Keybase widget showing grincoin#general chat #182

Merged
merged 4 commits into from
Feb 3, 2020

Conversation

nijynot
Copy link
Member

@nijynot nijynot commented Jan 23, 2020

This PR adds a Keybase chat widget showing the grincoin#general channel.

As Keybase does not have an open API to access team messages, this solution relies on me serving team message and member count from these two endpoints:

  • https://grin.nijynot.com/messages and
  • https://grin.nijynot.com/team.

I did not go with Chia's keybase-live-feed at the end as I feel it was a bit too complicated for a small chat widget, and it'd be faster to just build it from scratch (given that we'd want to change the UI). grin-keybase-chat is instead a Node.js server on a Digital Ocean droplet and the repo can be found on mimblewimble/keybase-chat-widget.

@lehnberg
Copy link
Contributor

Very cool @nijynot, well done as always!! 😀

Don't really have much to add aside from some nit-pick, here goes:

  • If you like, I'm happy to create a repo for the widget under /mimblewimble, might give it a bit more exposure and attention from other contributors. If you prefer to have it under you that's absolutely fine.
  • The project is generally Apache-2.0 but I notice the widget is MIT. Would you consider changing?
  • And kind of a side point to this PR, I have a nagging suspicion that with all the myriad keybase teams and channels we use, we suffer a major discoverability problem. It would be nice to have more of these widgets showing (for example for #dev, and for node and wallet teams), but not sure whether it's an overkill, and/or how we would structure it on the website? Do you have some thoughts about this?

once again, awesome work! 👏

@nijynot
Copy link
Member Author

nijynot commented Jan 24, 2020

Good points!

  1. I think it makes sense to makes sense to move the repo to mimblewimble. It’s made for the grin.mw site afterall.
  2. Yes can switch to Apache-2.0, no prob!
  3. Good idea. Thinking about adding a dropdown that lets you switch channels, between wallet general and community general and whatnot. What do you think?

@lehnberg
Copy link
Contributor

Awesome! Will just check with others that there are no new-repo-objections 👍

Thinking about adding a dropdown that lets you switch channels, between wallet general and community general and whatnot. What do you think?

Oh, that would be super! :D

@lehnberg
Copy link
Contributor

@nijynot repo created here, https://github.com/mimblewimble/keybase-chat-widget
I also added you to the site-dev team giving you commit rights to both /site and this repo. :)

Perhaps it makes sense to update this pr so that it points to the new repo before we merge?

@nijynot
Copy link
Member Author

nijynot commented Feb 1, 2020

Migrated over to the new repo and updated the link to point to it too.

@lehnberg
Copy link
Contributor

lehnberg commented Feb 1, 2020

Awesome! However, Github fired a security vulnerability alert in the repo based on serialize-javascript < 2.1.1 as defined in yarn.lock:

Remediation

Upgrade serialize-javascript to version 2.1.1 or later. For example:

serialize-javascript@^2.1.1:
  version "2.1.1"

Any chance we can fix this on your deployment before we merge it?

@lehnberg
Copy link
Contributor

lehnberg commented Feb 3, 2020

Awesome! However, Github fired a security vulnerability alert in the repo based on serialize-javascript < 2.1.1 as defined in yarn.lock:

Remediation

Upgrade serialize-javascript to version 2.1.1 or later. For example:

serialize-javascript@^2.1.1:
  version "2.1.1"

Any chance we can fix this on your deployment before we merge it?

Actually this shouldn't be a blocker I realise, sorry about that. It's not related to /site but to your deployment in the end and I will open an issue I the repo instead. LGTM 👍

@lehnberg lehnberg merged commit e9c54b9 into mimblewimble:master Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants