Skip to content

Conversation

jenbutongit
Copy link
Contributor

@jenbutongit jenbutongit commented Oct 31, 2018

#83
🤞hopefully this separates test and linting stages.

Also don't want to be running carthage twice here, just for the tests. Moved it into it's own lane rather than before_all

TBD: ignore_exit_status might have to be set to true if we want to enforce linting but there'll be some work to fix these: Done linting! Found 1173 violations, 202 serious in 130 files. 😧

@CLAassistant
Copy link

CLAassistant commented Oct 31, 2018

CLA assistant check
All committers have signed the CLA.

@chickdan
Copy link
Contributor

The reason I didn't include this fastlane step sooner is because we don't have a way to view the results of the PR's other people make. If we could get Danger (or something similar) added, that would be awesome!

@jenbutongit
Copy link
Contributor Author

@chickdan ok! looks good. looks like someone needs to set up a bot account & API key. Happy to do so but probably best in the hands of @benhalpern / @jessleenyc?

@jessleenyc
Copy link
Contributor

I think @maestromac might have some thoughts on this.

@maestromac
Copy link
Contributor

Hey all. I'm not too familiar with Swift development but would it make sense to integrate Hound or CodeClimate to this repository instead of combining linting with Travis?

@chickdan
Copy link
Contributor

chickdan commented Nov 1, 2018

Code Climate looks pretty robust, I think it would be a good idea to utilize it alongside Travis.

@jenbutongit
Copy link
Contributor Author

@maestromac is this all ok? I'm not familiar with CodeClimate. I assume all that needs to be done now is @thepracticaldev to switch on CodeClimate for this repo.

@maestromac
Copy link
Contributor

maestromac commented Nov 1, 2018

@jenbutongit Yup looks good. I just integrated CodeClimate to this repo. I will be pushing a blank commit to your work to attempt to trigger the webhook.

Also adding some badges

@maestromac maestromac requested a review from benhalpern November 1, 2018 20:17
@jenbutongit
Copy link
Contributor Author

hehe, since CC is turned on repo wide it looks like it's failing other builds. I'll add to the readme how to run the linter.

Copy link
Contributor

@maestromac maestromac left a comment

Choose a reason for hiding this comment

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

LGTM!

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

Successfully merging this pull request may close these issues.

5 participants