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

Convert ListVersions() to return []PairedVersion #210

Merged
merged 5 commits into from
Apr 15, 2017
Merged

Convert ListVersions() to return []PairedVersion #210

merged 5 commits into from
Apr 15, 2017

Conversation

sdboyer
Copy link
Owner

@sdboyer sdboyer commented Apr 6, 2017

This is almost complete, except one test is still failing in a way that
indicates the test harness still isn't quite dealing with the underlying
FAKEREVs correctly.

Fixes #202.

@@ -1456,20 +1462,32 @@ func (sm *depspecSourceManager) ListPackages(id ProjectIdentifier, v Version) (p
return pkgtree.PackageTree{}, fmt.Errorf("Project %s at version %s could not be found", pid.n, v)
}

func (sm *depspecSourceManager) ListVersions(id ProjectIdentifier) (pi []Version, err error) {
func (sm *depspecSourceManager) ListVersions(id ProjectIdentifier) ([]PairedVersion, error) {
pvl := make([]PairedVersion, 0)

Choose a reason for hiding this comment

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

can probably use "var pvl []PairedVersion" instead

@sdboyer
Copy link
Owner Author

sdboyer commented Apr 6, 2017

An enterprising soul could probably figure this one out and update this issue with a suggestion about where the problem is 😄

The goal within the test harness - in depspecSourceManager.ListVersions() - is to produce those paired versions with an underlying FAKEREV only if necessary to meet the interface requirement, and then strip them out immediately in depspecBridge.listVersions(). The latter is the only thing the solver should be calling...I think. Somehow, though, FAKEREVs are making it through into the solutions. The task here is to figure out where that leak is.

@codecov
Copy link

codecov bot commented Apr 15, 2017

Codecov Report

Merging #210 into master will decrease coverage by 3.18%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #210      +/-   ##
==========================================
- Coverage   78.02%   74.83%   -3.19%     
==========================================
  Files          28        4      -24     
  Lines        4018      465    -3553     
==========================================
- Hits         3135      348    -2787     
+ Misses        659       95     -564     
+ Partials      224       22     -202
Impacted Files Coverage Δ
selection.go
source_cache.go
deduce.go
rootdata.go
maybe_source.go
cmd.go
vcs_source.go
constraints.go
trace.go
manifest.go
... and 10 more

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 c7c4309...a9b5817. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 15, 2017

Codecov Report

Merging #210 into master will increase coverage by 0.39%.
The diff coverage is 96.79%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #210      +/-   ##
==========================================
+ Coverage   79.01%   79.41%   +0.39%     
==========================================
  Files          30       31       +1     
  Lines        4175     4143      -32     
==========================================
- Hits         3299     3290       -9     
+ Misses        649      624      -25     
- Partials      227      229       +2
Impacted Files Coverage Δ
satisfy.go 80.92% <100%> (ø) ⬆️
version.go 82.36% <100%> (+3.29%) ⬆️
solver.go 83.97% <100%> (+0.09%) ⬆️
bridge.go 65.38% <100%> (-10.54%) ⬇️
version_queue.go 100% <100%> (ø) ⬆️
selection.go 82.05% <100%> (ø) ⬆️
source_manager.go 91.23% <100%> (+0.88%) ⬆️
version_unifier.go 95.61% <95.61%> (ø)
pkgtree/pkgtree.go 81.4% <0%> (-3.67%) ⬇️
... and 1 more

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 df000f6...22be9f3. Read the comment docs.

sdboyer added 3 commits April 14, 2017 22:57
This is almost complete, except one test is still failing in a way that
indicates the test harness still isn't quite dealing with the underlying
FAKEREVs correctly.
@sdboyer sdboyer merged commit d193adb into master Apr 15, 2017
krisnova pushed a commit to krisnova/dep that referenced this pull request Apr 21, 2017
Convert ListVersions() to return []PairedVersion
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants