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 /who handling #4547

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add /who handling #4547

wants to merge 2 commits into from

Conversation

MaxLeiter
Copy link
Member

@MaxLeiter MaxLeiter commented Apr 27, 2022

irc-framework sends the WHOX cuhsnfdaor flags and doesn't support receiving others, so we manually remove parameters the user doesn't request.

I also added an uncaught exception handler on the server to stop a subset of crashes. The problem is people won't look at their server logs often, but bookworm and I agreed that's better than crashing. When we add an admin role we can report it to them.

https://ircv3.net/specs/extensions/whox

ref kiwiirc/irc-framework#331

@MaxLeiter MaxLeiter added the Type: Feature Tickets that describe a desired feature or PRs that add them to the project. label Apr 27, 2022
@MaxLeiter MaxLeiter added the Status: needs-review PR not yet reviewed by enough maintainers label Apr 28, 2022
@MaxLeiter MaxLeiter added this to the 4.3.2 milestone Apr 28, 2022
@MaxLeiter MaxLeiter added this to To do in IRCv3 via automation Apr 30, 2022
Copy link
Member

@itsjohncs itsjohncs left a comment

Choose a reason for hiding this comment

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

Looks like there's no help material being added and there's no error messages we give back from the API (though maybe the server is doing some of that for us). Are you planning on doing another pass after this lands to add help text and error messages?

Though I'm also not sure I understand why support for /who is being added.

I also added an uncaught exception handler on the server to stop a subset of crashes. The problem is people won't look at their server logs often, but bookworm and I agreed that's better than crashing.

I'll defer to you two. But TBH I'd prefer we continue to crash. I'd prefer failures make as much noise as possible especially with that huge TypeScript change looming.

src/plugins/inputs/who.js Outdated Show resolved Hide resolved
@brunnre8 brunnre8 added Status: changes-requested Review happened but some changes need to be made and removed Status: needs-review PR not yet reviewed by enough maintainers labels Jul 19, 2022
irc-framework sends the WHOX cuhsnfdaor flags and doesn't support receiving others, so we manually remove parameters the user doesn't request.

I also added an uncaught exception handler on the server to stop a subset of crashes. The problem is people won't look at their server logs often, but bookworm and I agreed that's better than crashing. When we add an admin role we can report it to them.

https://ircv3.net/specs/extensions/whox

ref kiwiirc/irc-framework#331
@MaxLeiter MaxLeiter added Status: needs-review PR not yet reviewed by enough maintainers and removed Status: changes-requested Review happened but some changes need to be made labels Sep 8, 2022
a: "account",
r: "realname",
};

Copy link
Member

Choose a reason for hiding this comment

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

Hm, why are you doing it this way?
You are creating a mapping that's never used, plus a function to translate it to what you actually want.

Can't we just directly create the mapping we need from single char to field name of TL?

@brunnre8 brunnre8 added Status: changes-requested Review happened but some changes need to be made and removed Status: needs-review PR not yet reviewed by enough maintainers labels Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: changes-requested Review happened but some changes need to be made Type: Feature Tickets that describe a desired feature or PRs that add them to the project.
Projects
IRCv3
  
To do
Development

Successfully merging this pull request may close these issues.

None yet

3 participants