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

ISSUE-16 - Remove cargo core usage from graph module: #136

Conversation

jmcconnell26
Copy link
Contributor

  • Pull further structs from Args struct
  • Add functions to krates_utils for replace and deps_not_replaced
    functions from resolve
  • Add function for matches_ignoring_source
  • Adding functions to extract the name and version of a cargo_metadata
    package from the PackageId
  • Remove usage of cargo core from graph module

Signed-off-by: joshmc [email protected]

@jmcconnell26
Copy link
Contributor Author

This is a work in progress.
I seem to be having issues with the tests for cargo-geiger-serde.
The output of cargo-geiger remains constant despite my changes, however passing the --json flag generates different output.
I have attached both the new and old output for the second test package here.
To me the new output actually looks correct, however I am unsure what the expected behaviour is.
@anderejd, @stephaneyfx, could you advise on what the correct output should be?
Thanks,
Josh

new.txt
old.txt

@stephaneyfx
Copy link
Contributor

Hi Josh,

AFAICT the old output is correct. Every package in the dependency tree rooted at test2_package_with_shallow_deps should have its own entry in the packages array in the json report. The new report contains only the root package of the dependency tree.

Does that help?

@jmcconnell26 jmcconnell26 force-pushed the ISSUE-16-RemoveCargoCoreUsageFromGraph branch from 905a18f to a842b7c Compare November 8, 2020 19:33
@jmcconnell26
Copy link
Contributor Author

Hi @stephaneyfx,

Ahh, I follow now, we aren't including any of the dependency information on the packages in the JSON report, that makes sense.
I'm not 100% why the changes I made caused this, but I'll take a look into this now.

Thanks for the response!
Josh

@jmcconnell26 jmcconnell26 force-pushed the ISSUE-16-RemoveCargoCoreUsageFromGraph branch 2 times, most recently from 32eacc2 to 84ee6f7 Compare November 9, 2020 18:38
* Pull further structs from `Args` struct
* Add functions to krates_utils for `replace` and `deps_not_replaced`
  functions from resolve
* Add function for `matches_ignoring_source`
* Adding functions to extract the name and version of a cargo_metadata
  package from the `PackageId`
* Remove usage of cargo core from `graph` module
* Pulling out parameter struct for walking dependency

Signed-off-by: joshmc <[email protected]>
@jmcconnell26 jmcconnell26 force-pushed the ISSUE-16-RemoveCargoCoreUsageFromGraph branch from 84ee6f7 to 15e6732 Compare November 9, 2020 19:52
@jmcconnell26
Copy link
Contributor Author

This PR is now ready for review. I was able to catch where I had gone wrong!
Thanks,
Josh

@anderejd anderejd merged commit 5fe50b5 into geiger-rs:master Nov 9, 2020
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.

3 participants