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

Display unhandled numerics on the client #286

Merged
merged 1 commit into from Jul 6, 2016
Merged

Display unhandled numerics on the client #286

merged 1 commit into from Jul 6, 2016

Conversation

xPaw
Copy link
Member

@xPaw xPaw commented Apr 27, 2016 •
edited

2016-04-24_18-31-52

@xPaw xPaw added Type: Feature Tickets that describe a desired feature or PRs that add them to the project. Meta: Do Not Merge This PR should not be merged. labels Apr 27, 2016
@xPaw xPaw mentioned this pull request May 12, 2016
Closed
@xPaw xPaw added this to the ★ Next Release milestone May 13, 2016
@xPaw xPaw removed the Meta: Do Not Merge This PR should not be merged. label Jul 2, 2016
@xPaw
Copy link
Member Author

xPaw commented Jul 2, 2016

Ready after #449

@maxpoulin64 maxpoulin64 mentioned this pull request Jul 2, 2016
Closed
@astorije
Copy link
Member

astorije commented Jul 3, 2016 •
edited

Reposting here after commenting on the wrong PR. Wow.

This is a nice addition, thanks!

One comment I have is: couldn't we wrap this behind a user setting, similar to the MOTD one? Not sure what name we could give it, but if it's a generic info checkbox, I would also put the "Enabled capabilities" there (if possible, of course).

Also (unrelated to this PR, but displayed by it), I noticed a line saying [403] Freenode No such channel, are we trying to join the lobby somewhere when first connecting? Fixed by #453.

@astorije astorije self-assigned this Jul 3, 2016
@xPaw
Copy link
Member Author

xPaw commented Jul 4, 2016

@astorije I don't think a setting for messages shown in server window is worth it.

@astorije
Copy link
Member

astorije commented Jul 6, 2016

Well, we'll have to deal with server window at some point still, because after this PR, we'll display everything or everything but the MOTD, which is the most human-readable information there lol.
But another fight, another day.

👍 and merging.

@astorije astorije merged commit b3d3582 into master Jul 6, 2016
@astorije astorije deleted the xpaw/unknown-command branch July 6, 2016 03:23
@astorije
Copy link
Member

astorije commented Jul 6, 2016

Damn, I just realized this wasn't in second review needed, sorry 😕
@maxpoulin64, @YaManicKill, are you 👍 on this a posteriori or do you want me to revert?

@maxpoulin64
Copy link
Member

@astorije: fine by me, 👍

matburnham pushed a commit to matburnham/lounge that referenced this pull request Sep 6, 2017
@astorije
Merge pull request thelounge#286 from thelounge/xpaw/unknown-command
afee449 
Display unhandled numerics on the client
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@astorije astorije

Labels
Type: Feature Tickets that describe a desired feature or PRs that add them to the project.
Projects
None yet
Milestone
2.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@xPaw @astorije @maxpoulin64