Show the difference between original in transformed flows in step detail

[anothermattbrown]

No description provided.

[batfish-bot]

This change is 

[codecov]

Codecov Report

Merging #2813 into master will increase coverage by 0.48%.
The diff coverage is 82.14%.

@@             Coverage Diff              @@
##             master    #2813      +/-   ##
============================================
+ Coverage     71.62%   72.11%   +0.48%     
- Complexity    19963    20329     +366     
============================================
  Files          1740     1743       +3     
  Lines         88116    90232    +2116     
  Branches      10720    11156     +436     
============================================
+ Hits          63116    65073    +1957     
- Misses        20492    20610     +118     
- Partials       4508     4549      +41

[progwriter]

Reviewed 5 of 5 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @anothermattbrown and @dhalperi)

---

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

@JsonTypeName("FlowDiff")
@ParametersAreNonnullByDefault
public final class FlowDiff implements Comparable<FlowDiff> {

Why does this need to implement comparable?

---

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

  /** Compute the differences between two flows */
  public static SortedSet<FlowDiff> flowDiffs(@Nullable Flow flow1, @Nullable Flow flow2) {

Why not a list? They order can be defined "naturally" in code (I think srcIp before dstIp for example is more natural), also get's rid of the need for comparable.

---

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

    }

    Preconditions.checkArgument(

don't qualify import

---

projects/batfish/src/main/java/org/batfish/dataplane/traceroute/TracerouteEngineImplContext.java, line 309 at r1 (raw file):

                        new NodeInterfacePair(currentNodeName, nextHopInterfaceName))
                    .setOutputFilter(outFilter != null ? outFilter.getName() : null)
                    .setFlowDiffs(

:/ make an issue that we need a separate NAT/transform step? It's really weird having it on the output.

[anothermattbrown]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @anothermattbrown, @progwriter, and @dhalperi)

---

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

Previously, progwriter (Victor Heorhiadi) wrote…

Why not a list? They order can be defined "naturally" in code (I think srcIp before dstIp for example is more natural), also get's rid of the need for comparable.

I used a list at first, but thought was the standard way we do handle set-like things (for which order is meaningless and there should not be duplicate elements).

---

projects/batfish/src/main/java/org/batfish/dataplane/traceroute/TracerouteEngineImplContext.java, line 309 at r1 (raw file):

Previously, progwriter (Victor Heorhiadi) wrote…

:/ make an issue that we need a separate NAT/transform step? It's really weird having it on the output.

done #2816

[anothermattbrown]

Reviewable status: 4 of 5 files reviewed, 2 unresolved discussions (waiting on @progwriter, @anothermattbrown, and @dhalperi)

---

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

Previously, progwriter (Victor Heorhiadi) wrote…

don't qualify import

done.

[progwriter]

Reviewable status: 4 of 5 files reviewed, 2 unresolved discussions (waiting on @progwriter, @anothermattbrown, and @dhalperi)

---

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

Previously, anothermattbrown (Matt Brown) wrote…

I used a list at first, but thought was the standard way we do handle set-like things (for which order is meaningless and there should not be duplicate elements).

Trying really hard to move away from this paradigm :)

[anothermattbrown]

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

---

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

Previously, progwriter (Victor Heorhiadi) wrote…

Trying really hard to move away from this paradigm :)

let's deal with that later :)

[progwriter]

Reviewed 1 of 1 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dhalperi)