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

feat(common): Add Path utilities for dSYM structures #212

Merged
merged 4 commits into from
Apr 7, 2020

Conversation

jan-auer
Copy link
Member

@jan-auer jan-auer commented Apr 7, 2020

Implements utilities for dealing with dSYM folder structures. They can check for a dSYM folder structure, resolve the dSYM parent from a deep path, and resolve the deep path from the parent folder.

sentry-cli partially implements this logic already, so we can reduce some duplicate code here. Also, the examples used to require that one declares the full path to a dSYM file, but this is just unnecessary.

_Users_jauer_Coding_symbolic_target_doc_symbolic_common_trait DSymPathExt html

@jan-auer jan-auer requested a review from a team April 7, 2020 09:01
@jan-auer jan-auer self-assigned this Apr 7, 2020
///
/// # Examples
///
/// ```no_run
Copy link
Member

Choose a reason for hiding this comment

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

oh, and I thought these were actually real tests. Would you mind changing them in a way that they use any of the fixtures that we have? Would be nice to have coverage here.

Copy link
Member Author

Choose a reason for hiding this comment

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

These are doc tests, so I cannot use our fixtures (and it also makes little sense). These docs are modelled after the std::path::Path docs.

However, I agree that this should be tested, let me add some tests.

examples/dump_cfi.rs Show resolved Hide resolved
Copy link
Contributor

@flub flub left a comment

Choose a reason for hiding this comment

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

LGTM though +1 on adding tests

@jan-auer jan-auer merged commit 63ebf6f into master Apr 7, 2020
@jan-auer jan-auer deleted the feat/example-dsym-path branch April 7, 2020 11:31
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.

None yet

3 participants