Skip to content

comm_command - #1179

Open
adrienntindall wants to merge 53 commits into
pret:mainfrom
adrienntindall:comm_command
Open

comm_command#1179
adrienntindall wants to merge 53 commits into
pret:mainfrom
adrienntindall:comm_command

Conversation

@adrienntindall

Copy link
Copy Markdown
Collaborator

Builds on #1161

This PR is long but ultimately has 3 goals:

  1. Fully document the contents of comm_command.c. This file should be reviewed thoroughly, and is thankfully short.

  2. Give general names and corresponding constants to each other comm_command_ file, named as such when they contain a CommCmdTable list of functions. These are not intended to be documented further and only the tables themselves need to be looked at when reviewing, since they will contain all already documented functions that had their names changed.

  3. Use the COMM_CMD_ constants in appropriate locations in order to maintain functionality when adding new communication functions to the base table. I'm pretty sure I found all instances, but if I missed any I would like that to mentioned as a part of this review.

@scbroede scbroede self-assigned this Jun 2, 2026

@scbroede scbroede left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

imo just like with script commands, these should all be named in the imperative sense (ie. verb-first, phrased as a command). aside from the ones i've individually flagged, commands not already following this convention should have Recv in front.

also this has conflicts

Comment thread include/constants/communication/comm_command.h Outdated
Comment thread src/comm_command_field.c Outdated
Comment thread src/comm_command_field.c Outdated
Comment thread src/comm_command_field.c Outdated
Comment thread src/comm_command_field.c Outdated
Comment thread src/comm_command_field.c Outdated
Comment thread src/comm_command_field.c Outdated
Comment thread src/comm_command_field.c Outdated
@adrienntindall

Copy link
Copy Markdown
Collaborator Author

imo just like with script commands, these should all be named in the imperative sense (ie. verb-first, phrased as a command). aside from the ones i've individually flagged, commands not already following this convention should have Recv in front.

also this has conflicts

I think that adding Recv to almost every comm command is gonna be too much, since as stated before they're implicitly receive functions already and that can get really cluttered really quickly.

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