Skip to content
This repository has been archived by the owner on Feb 3, 2018. It is now read-only.

Refactor reachmaps #148

Merged
merged 18 commits into from
Feb 22, 2017
Merged

Refactor reachmaps #148

merged 18 commits into from
Feb 22, 2017

Conversation

sdboyer
Copy link
Owner

@sdboyer sdboyer commented Jan 19, 2017

Fixes #113
Fixes #152
Fixes #127

@codecov-io
Copy link

codecov-io commented Jan 19, 2017

Codecov Report

Merging #148 into master will increase coverage by 1.69%.
The diff coverage is 79.42%.

@@            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
Impacted Files Coverage Δ
satisfy.go 80.92% <100%> (ø)
rootdata.go 92.1% <100%> (+0.1%)
solver.go 83.12% <78.12%> (ø)
analysis.go 86.49% <78.46%> (+1.18%)
trace.go 72.22% <90%> (ø)
vcs_source.go
source.go 73.33% <ø> (+34.15%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f8d2e5...4728f02. Read the comment docs.

)

var (
M = sort.Strings

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

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

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

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

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

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

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

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

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

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

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

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

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.
@sdboyer
Copy link
Owner Author

sdboyer commented Feb 21, 2017

All that's left now is a parameter for backpropagation and updating the docs.

@sdboyer
Copy link
Owner Author

sdboyer commented Feb 21, 2017

...oh but then also differentiating between missing pkgs and ignored pkgs, i guess?

@sdboyer sdboyer merged commit 5ede656 into master Feb 22, 2017
@sdboyer sdboyer deleted the reachmaps branch April 5, 2017 18:33
krisnova pushed a commit to krisnova/dep that referenced this pull request Apr 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants