Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix (readtree) correct the way we parse dependency graphs #385

Merged
merged 3 commits into from
Dec 11, 2018

Conversation

zlav
Copy link
Member

@zlav zlav commented Dec 6, 2018

readtree.go does not pop the top of the stack when parsing a dependency list which results in the initial node never being removed. This results in a slightly incorrect dependency graph.

Expected Behavior:

	├── dep:one:1.0.0
	└─┬ dep:two:2.0.0
	  ├─┬ dep:three:3.0.0
	  │ └── dep:four:4.0.0
	  └── dep:five:5.0.0

Current Behavior:

	├─┬ dep:one:1.0.0
	│ └── dep:five:5.0.0
	└─┬ dep:two:2.0.0
	  └─┬ dep:three:3.0.0
	    └── dep:four:4.0.0

This PR:

  1. Fixes readtree.go.
  2. Fix the other generated dependency parsers.
  3. Adds supporting tests inside of maven_test.go.

@zlav zlav changed the title Fix/maven graph traversal fix (readtree) correct the way we parse dependency graphs Dec 6, 2018
@zlav zlav requested a review from elldritch December 6, 2018 23:51
@codecov
Copy link

codecov bot commented Dec 7, 2018

Codecov Report

Merging #385 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #385   +/-   ##
=======================================
  Coverage   47.08%   47.08%           
=======================================
  Files          68       68           
  Lines        3158     3158           
=======================================
  Hits         1487     1487           
  Misses       1522     1522           
  Partials      149      149
Impacted Files Coverage Δ
buildtools/maven/readtree_generated.go 92.85% <100%> (ø) ⬆️
buildtools/composer/readtree_generated.go 85.71% <100%> (ø) ⬆️

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 1d39bb8...8ca7ea1. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Dec 7, 2018

Codecov Report

Merging #385 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #385   +/-   ##
=======================================
  Coverage   47.08%   47.08%           
=======================================
  Files          68       68           
  Lines        3158     3158           
=======================================
  Hits         1487     1487           
  Misses       1522     1522           
  Partials      149      149
Impacted Files Coverage Δ
buildtools/maven/readtree_generated.go 92.85% <100%> (ø) ⬆️
buildtools/composer/readtree_generated.go 85.71% <100%> (ø) ⬆️

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 1d39bb8...8ca7ea1. Read the comment docs.

@elldritch elldritch removed their assignment Dec 11, 2018
@zlav zlav merged commit 098b089 into master Dec 11, 2018
@zlav zlav deleted the fix/maven-graph-traversal branch December 11, 2018 07:35
meghfossa pushed a commit that referenced this pull request Nov 12, 2021
* bump tag

* changelog
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.

2 participants