Skip to content
This repository has been archived by the owner on Aug 23, 2020. It is now read-only.

Remove magic number from handshake #1694

Merged
merged 2 commits into from
Jan 6, 2020
Merged

Remove magic number from handshake #1694

merged 2 commits into from
Jan 6, 2020

Conversation

GalRogozinski
Copy link
Contributor

@GalRogozinski GalRogozinski commented Dec 19, 2019 •
edited
Loading

Description

While looking over networking code I found a magic number that confused me

Type of change

  • Readability fix

How Has This Been Tested?

If all unit tests and regression test pass it should suffice

Checklist:

  • My code follows the style guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • New and existing unit tests pass locally with my changes

@GalRogozinski
Copy link
Contributor Author

Quick explanation:
(maxLength - (maxLength - 60) + Protocol.SUPPORTED_PROTOCOL_VERSIONS.length = 60 + Protocol.SUPPORTED_PROTOCOL_VERSIONS.length

It was unclear where 60 comes from, so it was replaced constants

acha-bill
acha-bill approved these changes Dec 19, 2019
View reviewed changes
Copy link
Contributor

@acha-bill acha-bill left a comment

Choose a reason for hiding this comment

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

Great

@GalRogozinski GalRogozinski added the L-Test Passed Code should be merged label Dec 26, 2019
@GalRogozinski GalRogozinski merged commit 4ff8bae into iotaledger:dev Jan 6, 2020
@GalRogozinski GalRogozinski mentioned this pull request Jan 6, 2020
Open
@GalRogozinski GalRogozinski mentioned this pull request May 6, 2020
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@acha-bill acha-bill acha-bill approved these changes

Assignees
No one assigned
Labels
L-Test Passed Code should be merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@GalRogozinski @acha-bill