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

WIP: net/p2p:rename command*/Command/* to message*/Message* #24143

Conversation

RandyMcMillan
Copy link
Contributor

@RandyMcMillan RandyMcMillan commented Jan 25, 2022

-BEGIN VERIFY SCRIPT-
SED=$(which gsed)
export SED
echo $SED
export SRC=src
function s { git grep -l "$1" $SRC | xargs $SED -i "s/$1/$2/g"; }
function doit1 {
s 'COMMAND_SIZE' 'MESSAGE_SIZE'
s 'command name' 'message name'
s 'pchCommand' 'pchMessage'
s 'pszCommand' 'pszMessage'
}
function doit2 {
s 'GetCommand' 'GetMessage'
}
export SRC=src && doit1
export SRC=src/net.h && doit1
export SRC=src/net.cpp && doit1
export SRC=src/protocol.h && doit1
export SRC=src/protocol.cpp && doit1
export SRC=src/test/fuzz/protocol.cpp && doit1
export SRC=src/test/fuzz/protocol.cpp && doit2
export SRC=src/test/fuzz/process_messages.cpp && doit1
export SRC=src/test/fuzz/process_messages.cpp && doit2
-END VERIFY SCRIPT-

@RandyMcMillan
Copy link
Contributor Author

After reviewing PR #24141
It seems appropriate to rename COMMAND_SIZE to MESSAGE_SIZE

MESSAGE_SIZE better suggests the usage and scope of this variable.

@RandyMcMillan RandyMcMillan changed the title net/p2p:rename COMMAND_SIZE to MESSAGE_SIZE WIP: net/p2p:rename COMMAND_SIZE to MESSAGE_SIZE Jan 25, 2022
@RandyMcMillan RandyMcMillan marked this pull request as draft January 25, 2022 01:00
@RandyMcMillan RandyMcMillan force-pushed the 1643068742-rename-COMMAND_SIZE-to-MESSAGE_SIZE branch from 696f7d4 to a0b6c0b Compare January 25, 2022 01:42
@RandyMcMillan RandyMcMillan marked this pull request as ready for review January 25, 2022 01:42
@DrahtBot DrahtBot added the P2P label Jan 25, 2022
@RandyMcMillan RandyMcMillan force-pushed the 1643068742-rename-COMMAND_SIZE-to-MESSAGE_SIZE branch from a0b6c0b to 1bcf1f0 Compare January 25, 2022 01:55
@RandyMcMillan RandyMcMillan marked this pull request as draft January 25, 2022 03:33
@RandyMcMillan RandyMcMillan force-pushed the 1643068742-rename-COMMAND_SIZE-to-MESSAGE_SIZE branch from 1bcf1f0 to cdc7db7 Compare January 25, 2022 04:45
@RandyMcMillan RandyMcMillan marked this pull request as ready for review January 25, 2022 04:46
@RandyMcMillan RandyMcMillan force-pushed the 1643068742-rename-COMMAND_SIZE-to-MESSAGE_SIZE branch 2 times, most recently from bccebe2 to ec85970 Compare January 25, 2022 06:08
@RandyMcMillan RandyMcMillan changed the title WIP: net/p2p:rename COMMAND_SIZE to MESSAGE_SIZE WIP: net/p2p:rename command*/Command/* to message*/Message* Jan 25, 2022
@RandyMcMillan RandyMcMillan force-pushed the 1643068742-rename-COMMAND_SIZE-to-MESSAGE_SIZE branch 3 times, most recently from 62160ef to 58cc189 Compare January 25, 2022 07:04
@RandyMcMillan RandyMcMillan marked this pull request as draft January 25, 2022 07:46
@RandyMcMillan RandyMcMillan force-pushed the 1643068742-rename-COMMAND_SIZE-to-MESSAGE_SIZE branch 4 times, most recently from 5c410a0 to 2711a02 Compare January 25, 2022 18:27
-BEGIN VERIFY SCRIPT-
test gsed && SED=$(which gsed) && export SED || SED=$(which sed)
export SRC=src
function s { git grep -l "$1" $SRC | xargs $SED -i "s/$1/$2/g"; }
function doit1 {
 s 'COMMAND_SIZE' 'MESSAGE_SIZE'
 s 'command name' 'message name'
 s 'pchCommand' 'pchMessage'
 s 'pszCommand' 'pszMessage'
 s 'IsCommandValid' 'IsMessageValid'
}
function doit2 {
 s 'GetCommand' 'GetMessage'
}
export SRC=src && doit1
export SRC=src/net.h && doit1
export SRC=src/net.cpp && doit1
export SRC=src/protocol.h && doit1
export SRC=src/protocol.cpp && doit1
export SRC=src/test/fuzz/protocol.cpp && doit1
export SRC=src/test/fuzz/protocol.cpp && doit2
export SRC=src/test/fuzz/process_messages.cpp && doit1
export SRC=src/test/fuzz/process_messages.cpp && doit2
-END VERIFY SCRIPT-
@RandyMcMillan RandyMcMillan force-pushed the 1643068742-rename-COMMAND_SIZE-to-MESSAGE_SIZE branch from 2711a02 to 34a1d3b Compare January 25, 2022 19:16
@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #23438 (refactor: Use spans of std::byte in serialize by MarcoFalke)
  • #23233 (BIP324: Add encrypted p2p transport {de}serializer by dhruv)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@laanwj
Copy link
Member

laanwj commented Jan 27, 2022

I do think "Message" is a better fit than "Command" conceptually. A P2P network doesn't have commands.
~0 on renaming it all over the place, though.
Also, maybe something like "MessageType" would be a better name when you're renaming anyway. It'd be clearer that it refers to a field defining what kind of message it is instead of the entire message.

@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".

@bitcoin bitcoin locked and limited conversation to collaborators Mar 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants