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

Improve error messages and handling of cargo manifest #38

Merged
merged 7 commits into from
Jun 15, 2019
Merged

Conversation

hmvp
Copy link
Contributor

@hmvp hmvp commented Jun 8, 2019

  • Fixed errror messages when not in cargo project to fix Fails to parse JSON when it's not a rust project #37

  • Refactor to use cargo metadata instead of cargo read-manifest (deprecated)
    This also gives us some workspace support (you need to choose a target from workspace)
    Improves Support for cargo workspaces #8 but only by not crashing on workspaces. We still show one target.

    The current assumption of one lib max fails when running in a workspace.
    Since the current code does not allow to select a specific lib target

    Also there are more target kinds than bin and lib that might be interesting to parse.

    We should revist the user experience here or always show the whole workspace.

Hiram van Paassen and others added 5 commits June 8, 2019 11:25
…ated)

This also gives us some workspace support (you need to choose a target from workspace)
Improves #8 but only by not crashing on workspaces. We still show one target.

The current assumption of one lib max fails when running in a workspace.
Since the current code does not allow to select a specific lib target

Also there are more target kinds than bin and lib that might be interesting to parse.

We should revist the user experience here or always show the whole workspace.
@hmvp hmvp requested a review from regexident June 8, 2019 20:40
src/manifest.rs Outdated Show resolved Hide resolved
src/manifest.rs Outdated Show resolved Hide resolved
Cargo versions might not generated the same order of targets or
similar issues leading to hard to fix test failures
@hmvp hmvp requested a review from muhuk June 11, 2019 19:53
Copy link
Contributor

@muhuk muhuk left a comment

Choose a reason for hiding this comment

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

This is going to mess up my long running branch but I'm willing to rebase/redo. I think overall this is a good patch.

Thanks for removing the system call from the unit tests. Also thanks for removing muhuk directory from the manifests. It was bothering me a bit.

Copy link
Owner

@regexident regexident left a comment

Choose a reason for hiding this comment

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

Great work!

@hmvp hmvp merged commit b6e79b2 into master Jun 15, 2019
@hmvp hmvp deleted the fix_37 branch June 15, 2019 16:04
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.

Fails to parse JSON when it's not a rust project
3 participants