-
Notifications
You must be signed in to change notification settings - Fork 229
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
EOS: parse peer-address heartbeat VRF #8426
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @arifogel and @ratulm)
projects/batfish/src/main/antlr4/org/batfish/grammar/arista/Arista_mlag.g4
line 28 at r1 (raw file):
: PEER_ADDRESS HEARTBEAT ip = IP_ADDRESS (VRF vrf_name)? NEWLINE ;
No two rules can start with the same prefix -- it's not LL1.
Have eos_mlag_peer_address
call 2 subrules based on first token.
Code quote:
PEER_ADDRESS ip = IP_ADDRESS NEWLINE
;
eos_mlag_peer_address_heartbeat
:
PEER_ADDRESS HEARTBEAT ip = IP_ADDRESS (VRF vrf_name)? NEWLINE
;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 2 files at r2, all commit messages.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @arifogel, @dhalperi, and @ratulm)
projects/batfish/src/main/antlr4/org/batfish/grammar/arista/Arista_mlag.g4
line 27 at r2 (raw file):
| eos_mpa_heartbeat ) NEWLINE
Rules should be tail-recursive unless there's a great reason not to. Aka, newlines are normally in child rules.
Is there a reason to deviate here?
Code quote:
NEWLINE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @arifogel and @ratulm)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @arifogel)
Codecov Report
@@ Coverage Diff @@
## master #8426 +/- ##
============================================
- Coverage 75.19% 75.18% -0.01%
Complexity 43944 43944
============================================
Files 3403 3403
Lines 168790 168795 +5
Branches 20037 20038 +1
============================================
- Hits 126914 126905 -9
- Misses 32314 32327 +13
- Partials 9562 9563 +1
|
Extract and ignore the optional VRF specification. The non-optional IP address part was already being ignored.