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

make SwiftLintViolation properties public #377

Merged
merged 3 commits into from
Sep 23, 2020
Merged

make SwiftLintViolation properties public #377

merged 3 commits into from
Sep 23, 2020

Conversation

feighter09
Copy link
Contributor

@feighter09 feighter09 commented Sep 21, 2020

In our project, we use swiftlint to flag todos as warnings in addition to style warnings. For todos, we have a limit of ~10 before we want to block the build, while we don't want any style / compiler warnings allowed, so I'd like to write a danger rule like so:

let violations = SwiftLint.lint(.all(directory: nil))
let todosLimit = 10

let todos = violations.filter { $0.ruleID == "todo_rule" }
let warnings = violations.count - todos.count

if warnings.count > 0 {
    fail("Warning count exceeds \(warningsLimit) warnings")
}
if todos.count > todosLimit {
    fail("todo count exceeds \(todosLimit) todos")
}

but currently cannot as ruleID is marked internal

public internal(set) var reason: String
public internal(set) var line: Int
public internal(set) var severity: Severity
public internal(set) var file: String

var messageText: String {
Copy link
Member

Choose a reason for hiding this comment

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

I think this also can be public 🤔

Copy link
Member

Choose a reason for hiding this comment

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

I think this is mainly for the PR message, so given they are automatically posted I think this can stay internal

Copy link
Member

@f-meloni f-meloni left a comment

Choose a reason for hiding this comment

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

Thank you for the PR, I think this is a reasonable change, we are returning the violations so makes sense to make them usable

public internal(set) var reason: String
public internal(set) var line: Int
public internal(set) var severity: Severity
public internal(set) var file: String

var messageText: String {
Copy link
Member

Choose a reason for hiding this comment

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

I think this is mainly for the PR message, so given they are automatically posted I think this can stay internal

@f-meloni f-meloni merged commit 6ef9adc into danger:master Sep 23, 2020
@feighter09 feighter09 deleted the af8-make-swiftlint-details-public branch September 23, 2020 13:20
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