-
Notifications
You must be signed in to change notification settings - Fork 24
Refactor reachmaps #148
Refactor reachmaps #148
Conversation
Codecov Report
@@ Coverage Diff @@
## master #148 +/- ##
==========================================
+ Coverage 78.88% 80.58% +1.69%
==========================================
Files 24 23 -1
Lines 3709 3652 -57
==========================================
+ Hits 2926 2943 +17
+ Misses 582 523 -59
+ Partials 201 186 -15
Continue to review full report at Codecov.
|
) | ||
|
||
var ( | ||
M = sort.Strings |
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.
exported var M should have comment or be unexported
import "hash" | ||
|
||
var ( | ||
H = hash.Hash |
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.
exported var H should have comment or be unexported
|
||
var ( | ||
_ = parser.ParseFile | ||
S = gps.Prepare |
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.
exported var S should have comment or be unexported
) | ||
|
||
var ( | ||
V = os.FileInfo |
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.
exported var V should have comment or be unexported
) | ||
|
||
var ( | ||
A = simple.A |
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.
exported var A should have comment or be unexported
) | ||
|
||
var ( | ||
A = gps.Solver |
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.
exported var A should have comment or be unexported
) | ||
|
||
var ( | ||
A = relimport.A |
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.
exported var A should have comment or be unexported
) | ||
|
||
var ( | ||
A = sort.Strings |
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.
exported var A should have comment or be unexported
) | ||
|
||
var ( | ||
A = sort.Strings |
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.
exported var A should have comment or be unexported
) | ||
|
||
var ( | ||
A = gps.Solve |
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.
exported var A should have comment or be unexported
) | ||
|
||
var ( | ||
A = gps.Solve |
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.
exported var A should have comment or be unexported
) | ||
|
||
var ( | ||
A = gps.Solve |
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.
exported var A should have comment or be unexported
4b3d66d
to
0dbe923
Compare
6fd33b5
to
4284958
Compare
Not covering this basic aspect of how real project roots actually work allowed a windows bug to hide until real data came through - #146.
This was never used, because it wasn't actually needed.
ExternalReach (to be renamed presently) was producing only a list of the externally reachable packages from the input set. This is the most important requirement, but there's also a need to keep track of which internal packages are (transitively) imported within each project. Without returning an internal reachmap - a list of all internal packages reachable from each other internal package - we end up recording only those packages from projects that were directly imported across project boundaries. Those packages that are only imported by other internal packages are missed.
Fixes a dumb error, but there's still an intermittent problem. Testing failures suggest a random map iteration order issue.
The old logic was a holdover from before a proper depth-first search algorithm was implemented in wmToReach(). It was trying to do a "bit" of the search work before the real algorithm; however, the final else statement was causing some internal imports to be dropped if the referent had already been visited. The bug was intermittent, as it depended on map iteration order. This also solves the secondary problem of inaccurate backpropagation/poisoning in wmToReach itself, as the internal linkage data it's operating on is now reliable.
Rather than splitting the data into two separate map return values, this makes ReachMaps' value a struct containing both the internal package import and external import path list information.
Much better name. Also adds the capability of filtering out stdlib from PackageTree imports, addressing #113.
This second parameter provides information about why a package was dropped from the ReachMap - either what problem it had itself, or the problem in one of the internal packages it transitively imports.
All that's left now is a parameter for backpropagation and updating the docs. |
...oh but then also differentiating between missing pkgs and ignored pkgs, i guess? |
Refactor reachmaps
Fixes #113
Fixes #152
Fixes #127