DRAFT : Adding functionality to post code coverage #5
Conversation
1. Adds a enum to specify which files should show warnings using Danger 2. Add code to generates list of file names from the options 3. Add code to filter warning issues based on file names list generate above.
1. adds 'failIfCoverageUnavailable' to allow user to fail the build if coverage is'nt avaiable 2. adds 'excludeCoverageTarget' to allow the user to filter out specific targets like pods or tests 3. adds 'showCoverageForChangedFiles' to allow user to show coverage for code changes and 'excludeCoverageForFiles' to exclude specific files 4. add code to support the above config vairables we first filter out the exculded targets, then exludedFiles. After which we proceed to post the Overall coverage following the coverage for diff files grouped by target. yumemi-inc#4
theScud
changed the title
|
@theScud Sorry, please let me review it at weekend! Still busy on company work |
|
no worries i still need to test this out, please take your time |
| } | ||
|
|
There was a problem hiding this comment.
[nits]
Please don't remove these blank lines. I made them for better readability.
There was a problem hiding this comment.
cool, would it possible to have a swift format file to the repo, would make it easier to make sure that we stick to convention without any misses
There was a problem hiding this comment.
Will add it later this week 👍
| reportingFileType: ReportingFileType = .all | ||
| excludeCoverageTarget: [ExcludedTarget] = [], | ||
| excludeCoverageForFiles: @escaping ((RelativeFilePath) -> Bool) = { _ in return false }, | ||
| showCoverageForChangedFiles: Bool = true |
There was a problem hiding this comment.
[must]
I Think it's probably too complicated now.
shouldFailIfCoverageUnavailable is only needed when codeCoverageRequirement is .required, but it's required even when codeCoverageRequirement is .none
Also excludeCoverageForFiles and showCoverageForChangedFiles are kind of ambiguous that users may feel difficult to determine what exactly they're doing.
My suggestion is
- Make a new
CodeCoverageReportSettingstype - Move
CoverageThresholdinto that type - Make properties like
excludeCoverageTargetsinside that type - Make a
defaultinitializer forCodeCoverageReportSettings - Change
CodeCoverageRequirement.required(CoverageThreshold)toCodeCoverageRequirement.required(CodeCoverageReportSettings)
Properties like parseBuildWarnings and parseBuildErrors may also be moved to another BuildSummaryReportSettings type, but that would be another ticket to fix
|
@el-hoshino i am a little tied up with work currently , i'll post the changes in the first week of November, apologies for the delay |
|
@theScud Don't worry! Actually I'm also occupied until end of this month; by then I may not be able to review your PR soon either 😅 |
DO NOT MERGE, current in testing.