Skip to content

1.0.rc1#82

Merged
jotuel merged 19 commits intomainfrom
111
Sep 22, 2025
Merged

1.0.rc1#82
jotuel merged 19 commits intomainfrom
111

Conversation

@jotuel
Copy link
Copy Markdown
Collaborator

@jotuel jotuel commented Sep 21, 2025

This should be ok to evaluate. To me the bot could use a Bot.cpp & Bot.hpp so a class which constructor initializes the connection with the server. And many of those helper functions could be private methods. Other thing is whether the macro usage helps or hurts readability. For me it helps as there are now less characters and more cases in Command.cpp and highlighting makes it easier on the eyes.

I think the Server class still needs to close all the remaining fd's in _clients map. Even though they are closed by the operating system this is just for school's sake. Maybe the setLimit also needs the stoul positions is null trick for tricky tricksters. This is release candidate 1 for a reason so comment the code and discuss. 👍🏼

Copy link
Copy Markdown
Collaborator

@v-kuu v-kuu left a comment

Choose a reason for hiding this comment

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

Nice work and useful bot. I tested it with various edge cases and everything seems to be handled correctly, except that it doesn't update it's internal list of operators for a channel when an operator leaves the channel. Minor issue though

@jotuel jotuel merged commit 231a587 into main Sep 22, 2025
2 checks passed
@jotuel jotuel deleted the 111 branch September 22, 2025 11:57
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.

3 participants