Skip to content

Junos: handle empty dhcp-relay server-groups#9052

Merged
dhalperi merged 2 commits intomasterfrom
empty-server-group
Jun 4, 2024
Merged

Junos: handle empty dhcp-relay server-groups#9052
dhalperi merged 2 commits intomasterfrom
empty-server-group

Conversation

@anothermattbrown
Copy link
Copy Markdown
Contributor

No description provided.

@batfish-bot
Copy link
Copy Markdown

This change is Reviewable

Copy link
Copy Markdown
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @anothermattbrown and @arifogel)


projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_forwarding_options.g4 line 82 at r1 (raw file):

fod_server_group
:
   SERVER_GROUP name = junos_name (address = ip_address)?

generally, would prefer new child rule with enter/exit. better than ? and complicated logic in extractor.

it's not striclty necessary, but so much of our maintainability (and parser recovery) pain is from not doing this kind of thing.

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.53%. Comparing base (934f071) to head (6f0e214).

Current head 6f0e214 differs from pull request most recent head 92de06c

Please upload reports for the commit 92de06c to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9052      +/-   ##
==========================================
+ Coverage   72.52%   72.53%   +0.01%     
==========================================
  Files        3324     3324              
  Lines      169743   169743              
  Branches    19926    19927       +1     
==========================================
+ Hits       123105   123128      +23     
+ Misses      37478    37455      -23     
  Partials     9160     9160              
Files Coverage Δ
...fish/grammar/flatjuniper/ConfigurationBuilder.java 68.40% <100.00%> (+0.37%) ⬆️

... and 3 files with indirect coverage changes

Copy link
Copy Markdown
Contributor Author

@anothermattbrown anothermattbrown left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 5 files reviewed, all discussions resolved (waiting on @arifogel and @dhalperi)


projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_forwarding_options.g4 line 82 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

generally, would prefer new child rule with enter/exit. better than ? and complicated logic in extractor.

it's not striclty necessary, but so much of our maintainability (and parser recovery) pain is from not doing this kind of thing.

done

Copy link
Copy Markdown
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @arifogel)

@dhalperi dhalperi enabled auto-merge (squash) June 4, 2024 22:57
@dhalperi dhalperi merged commit 35a864f into master Jun 4, 2024
@dhalperi dhalperi deleted the empty-server-group branch June 4, 2024 23:01
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.

3 participants