Refactor code for readability [redux] #181
Open
+277
−182
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
QuentinI
changed the title
The code in the `optimism_test_helpers.go` works but a lot of it is deeply nested and long which hurts readability. This change refactors the code into smaller functions with reduced indentation level, and more annotated comments to help readability, maintainability, and clarity.
When attempting to run the refactor on a linux machine, some errors are encountered. When Asking @philippecamacho about the errors encountered he provided the error: `"unable to determine the host for the espresso-dev-node sequencer api"` This error occurs when the remapping has failed. Comparing the refactor changes against the original implementation indicated a slight change in the logic. The change is meant to ensure that the port is populated correctly on the container port map. However it only checks the remapped values which are always populated with the given value. This makes this condition a tautological expression which will never remap when it should. This fix changes the inspected value, and also adds a fail-safe to always process the re-assignment when the `network` is set to `"host"`.
Ayiga
force-pushed
the
ag/ts/ref/cleanup-code-helpers-for-espresso-devnet
branch
from
b1b8d05 to
f125272
Compare
Ayiga
force-pushed
the
ag/ts/ref/cleanup-code-helpers-for-espresso-devnet
branch
from
0ab15cd to
d522613
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a re-base of #109
This PR:
Refactors the optimism_espresso_test_helpers.go file to help readability and maintainability
The code within the optimism_espresso_test_helpers.go file works without issue, but it can be difficult to follow due to being over a hundred lines and several indentation levels deep. This code has been split up into smaller functions that are more focused and feature additional comments to motivate and describe their actions.
This PR does not:
Modify the existing behavior, it should just move the logic and behavior into different functions to help readability and maintainability.