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

Add the option to enable/disable the main thread checker from the generated schemes #1382

Merged
merged 6 commits into from May 27, 2020

Conversation

pepicrft
Copy link
Contributor

Resolves #1321

Short description 📝

As @gbasile pointed out here we are enabling the main thread checker by default. This PR adds a new API to the RunAction and TestAction model to set diagnostics options.

@pepicrft pepicrft requested a review from a team May 27, 2020 11:53
@pepicrft pepicrft self-assigned this May 27, 2020
@pepicrft pepicrft requested review from andreacipriani and natanrolnik and removed request for a team May 27, 2020 11:53
@codecov
Copy link

codecov bot commented May 27, 2020

Codecov Report

Merging #1382 into master will increase coverage by 0.03%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1382      +/-   ##
==========================================
+ Coverage   75.08%   75.11%   +0.03%     
==========================================
  Files         317      318       +1     
  Lines       10391    10418      +27     
==========================================
+ Hits         7802     7826      +24     
- Misses       2589     2592       +3     
Impacted Files Coverage Δ
...ppers/SchemeDiagnosticsOption+ManifestMapper.swift 0.00% <0.00%> (ø)
Sources/ProjectDescription/RunAction.swift 100.00% <100.00%> (ø)
Sources/ProjectDescription/TestAction.swift 100.00% <100.00%> (ø)
Sources/TuistCore/Models/RunAction.swift 100.00% <100.00%> (ø)
Sources/TuistCore/Models/TestAction.swift 100.00% <100.00%> (ø)
...es/TuistGenerator/Generator/SchemesGenerator.swift 93.63% <100.00%> (+0.26%) ⬆️
...s/TuistKit/ProjectEditor/ProjectEditorMapper.swift 98.92% <100.00%> (ø)
...els+ManifestMappers/RunAction+ManifestMapper.swift 100.00% <100.00%> (ø)
...ls+ManifestMappers/TestAction+ManifestMapper.swift 90.47% <100.00%> (+1.00%) ⬆️

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 cc7f3ac...88c7f74. Read the comment docs.

@@ -269,6 +278,7 @@ final class SchemesGenerator: SchemesGenerating {

let onlyGenerateCoverageForSpecifiedTargets = codeCoverageTargets.count > 0 ? true : nil

let disableMainThreadChecker = !testAction.diagnosticsOptions.contains(.mainThreadChecker)
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought about using the affermative (enable) instead of negative (disable) disableMainThreadChecker -> isMainThreadCheckerEnabled since I think that's more readable. But I see that's the name used in the XCScheme

Copy link
Contributor Author

Choose a reason for hiding this comment

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

disableMainThreadChecker is how Xcode calls the attribute 😛 . That's why I took the opportunity to make it positive from our API . mainThreadChecker

@pepicrft pepicrft merged commit dac7cb4 into master May 27, 2020
@pepicrft pepicrft deleted the main-thread-checker branch May 27, 2020 17:19
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.

Disable Main thread checker
2 participants