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

Use Danger swift #13

Merged
merged 9 commits into from Oct 24, 2017

Conversation

Projects
None yet
3 participants
@orta
Member

orta commented Oct 24, 2017

I'm a tad wary of the danger in post ( it might mean the build can't be failed ) on CI, but this moves everything but Swift Lint checks into Danger Swift.

You could probably write a quick JSON -> warns thing in the bottom of the file for danger-swift, like the old v1 version of the ruby plugin.

ashfurrow added some commits Oct 24, 2017

@orta

This comment has been minimized.

Show comment
Hide comment
@orta

orta Oct 24, 2017

Member

hah - that fail message - danger/danger-js#408

Member

orta commented Oct 24, 2017

hah - that fail message - danger/danger-js#408

@MoyaBot

This comment has been minimized.

Show comment
Hide comment
@MoyaBot

MoyaBot Oct 24, 2017

Warnings
⚠️
[
{
  "rule_id" : "force_unwrapping",
  "reason" : "Force unwrapping should be avoided.",
  "character" : 163,
  "file" : "\/Users\/distiller\/Harvey\/Sources\/Harvey\/HarveyResponse.swift",
  "severity" : "Warning",
  "type" : "Force Unwrapping",
  "line" : 10
}
]

Generated by 🚫 dangerJS

MoyaBot commented Oct 24, 2017

Warnings
⚠️
[
{
  "rule_id" : "force_unwrapping",
  "reason" : "Force unwrapping should be avoided.",
  "character" : 163,
  "file" : "\/Users\/distiller\/Harvey\/Sources\/Harvey\/HarveyResponse.swift",
  "severity" : "Warning",
  "type" : "Force Unwrapping",
  "line" : 10
}
]

Generated by 🚫 dangerJS

@orta

This comment has been minimized.

Show comment
Hide comment
@orta

orta Oct 24, 2017

Member

OK, that's also got a simple version of Danger Swiftlint in it - ready to go.

Plus that warning was there before this PR

Member

orta commented Oct 24, 2017

OK, that's also got a simple version of Danger Swiftlint in it - ready to go.

Plus that warning was there before this PR

@ashfurrow

Looks good, I'll make the change I mentioned in the comments.

Show outdated Hide outdated Dangerfile.swift

@ashfurrow ashfurrow merged commit cb5fb5f into master Oct 24, 2017

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@ashfurrow

This comment has been minimized.

Show comment
Hide comment
@ashfurrow

ashfurrow Oct 24, 2017

Member

Thanks @orta!

Member

ashfurrow commented Oct 24, 2017

Thanks @orta!

@ashfurrow ashfurrow deleted the danger-swift branch Oct 24, 2017

dependencies:
override:
- brew install swiftlint
- npm install -g danger@alpha

This comment has been minimized.

@orta

orta Nov 15, 2017

Member

For anyone using this as a reference, you should use npm install -g danger instead now that Danger JS 2.0 is out 👍

@orta

orta Nov 15, 2017

Member

For anyone using this as a reference, you should use npm install -g danger instead now that Danger JS 2.0 is out 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment