8000 Add file path to json output by cconstable · Pull Request #178 · fastlane-community/xcov · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add file path to json output #178

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

Merged
merged 1 commit into from
Aug 18, 2020

Conversation

cconstable
Copy link
Contributor
@cconstable cconstable commented May 11, 2020

What

Adds the file path when generating json reports (via --json_report).

Old output

"files": [
  { 
    "name": "MyFile.swift",
    "coverage": 0,
    ...

New output

"files": [
  { 
    "name": "MyFile.swift",
    "path":"/Users/me/project/MyFile.swift",
    "coverage": 0,
    ...

Why

To assist automated tooling, it's helpful to know the path of the files. In larger projects, it's not uncommon to have duplicate file names across targets. Reconciling the coverage name with the actual file requires either a) parsing the Xcode coverage reports ourselves or b) keeping track of the corresponding target and parsing Xcode project files. Both options seem pretty unfortunate.

Open questions

  • Should this be gated behind a flag?
    • Opinion: Adding this field will increase the size of the generated json reports but I don't think the amount will be significant (esp after compression). Since this is an additive change, it is unlikely to break existing tooling.
  • Should this be added to any other reports?
    • Opinion: I'm assuming HTML reports are used mostly for humans and JSON reports are used mostly for tooling. If nobody is asking for it I don't see a reason to add it to the HTML reports.

@cconstable cconstable changed the title [Draft] Add file path to json output Add file path to json output Jun 1, 2020
@cconstable
Copy link
Contributor Author

Hey @joshdholtz, hope you are staying safe! Do you have any thoughts on this? If you feel additional changes are needed I would be happy to make them.

@joshdholtz
Copy link
Member

@cconstable Ah, sorry I missed this! I appreciate the ping 😊 This looks good! I'll merge and get a new version out 💪

Copy link
Member
@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

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

🔥 Sorry about not seeing this sooner! I'll get a new version out with this 💪

@joshdholtz joshdholtz merged commit b43bc24 into fastlane-community:master Aug 18, 2020
@joshdholtz
Copy link
Member

@cconstable 1.7.4 is release!

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
0