Skip to content

IPv6: fix toString#9246

Merged
dhalperi merged 1 commit intomasterfrom
spr/master/a3da853d
Oct 14, 2024
Merged

IPv6: fix toString#9246
dhalperi merged 1 commit intomasterfrom
spr/master/a3da853d

Conversation

@dhalperi
Copy link
Copy Markdown
Member

@dhalperi dhalperi commented Oct 14, 2024

Stack:

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.

@batfish-bot
Copy link
Copy Markdown

This change is Reviewable

@dhalperi dhalperi force-pushed the spr/master/a3da853d branch 2 times, most recently from d5ec156 to 9cbe894 Compare October 14, 2024 17:18
Copy link
Copy Markdown
Member Author

@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.

Reviewable status: 0 of 9 files reviewed, all discussions resolved (waiting on @SLarkworthy)


projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/Ip6.java line 50 at r1 (raw file):

      remainder = remainder.shiftRight(16);
    }

I feel like this should be simplifiable, but you know it wasn't easy to see how.

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 14, 2024

Codecov Report

Attention: Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.

Project coverage is 72.73%. Comparing base (fce5773) to head (6260fc3).
Report is 46 commits behind head on master.

Files with missing lines Patch % Lines
...tocol/src/main/java/org/batfish/datamodel/Ip6.java 96.55% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #9246   +/-   ##
=======================================
  Coverage   72.72%   72.73%           
=======================================
  Files        3316     3316           
  Lines      170084   170107   +23     
  Branches    20051    20061   +10     
=======================================
+ Hits       123702   123735   +33     
+ Misses      37216    37207    -9     
+ Partials     9166     9165    -1     
Files with missing lines Coverage Δ
...tocol/src/main/java/org/batfish/datamodel/Ip6.java 80.89% <96.55%> (+5.14%) ⬆️

... and 7 files with indirect coverage changes

Base automatically changed from spr/master/4467c014 to master October 14, 2024 18:06
@dhalperi dhalperi force-pushed the spr/master/a3da853d branch from 9cbe894 to b256c87 Compare October 14, 2024 18:08
@dhalperi dhalperi enabled auto-merge (squash) October 14, 2024 18:10
Copy link
Copy Markdown
Contributor

@SLarkworthy SLarkworthy 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: 0 of 9 files reviewed, 1 unresolved discussion (waiting on @dhalperi)


projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/Ip6.java line 78 at r1 (raw file):

    }

    // Handle edge case where the longest sequence is at the end

Can just move this check to right after you append currentLength and you wouldn't need to handle this case separately.

commit-id:a3da853d
@dhalperi dhalperi force-pushed the spr/master/a3da853d branch from b256c87 to 6260fc3 Compare October 14, 2024 18:43
Copy link
Copy Markdown
Member Author

@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.

Reviewable status: 0 of 9 files reviewed, 1 unresolved discussion (waiting on @SLarkworthy)


projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/Ip6.java line 78 at r1 (raw file):

Previously, SLarkworthy (Sarah Larkworthy) wrote…

Can just move this check to right after you append currentLength and you wouldn't need to handle this case separately.

Done.

Copy link
Copy Markdown
Contributor

@SLarkworthy SLarkworthy left a comment

Choose a reason for hiding this comment

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

Reviewed 8 of 9 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dhalperi)

@dhalperi dhalperi merged commit b0ca33b into master Oct 14, 2024
@dhalperi dhalperi deleted the spr/master/a3da853d branch October 14, 2024 19:09
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