Skip to content
This repository has been archived by the owner on Jan 4, 2020. It is now read-only.

Add Danger #102

Open
cojoj opened this issue Jun 7, 2017 · 11 comments
Open

Add Danger #102

cojoj opened this issue Jun 7, 2017 · 11 comments

Comments

@cojoj
Copy link
Contributor

cojoj commented Jun 7, 2017

Our CI is printing linter errors/warnings just fine, but the results aren't visible to the PR creators. - which kinda sucks, right? 😜
Do you know how to achieve this @pixyzehn?
One thing that comes to my mind is to introduce Danger (since buddybuild uses it without any issues) and use something like this.

@pixyzehn
Copy link
Contributor

pixyzehn commented Jun 7, 2017

@cojoj You can see the errors/warnings in buddybuild logs for now like this.
https://dashboard.buddybuild.com/public/apps/58ff19a79a06210001d14c2d/build/5937a686f8735a0001c4b64b#logs

But I know it's a bit troublesome... if we put a high priority on fixing the errors/warnings by SwiftLint, it's better to use such a tool.
The one bad thing is that we have to maintenance such a dependency.
@JohnSundell What do you think?

@cojoj
Copy link
Contributor Author

cojoj commented Jun 7, 2017

Hah, dependencies... Dependencies everywhere... ¯_(ツ)_/¯

@pixyzehn
Copy link
Contributor

pixyzehn commented Jun 7, 2017

Yes, but I understand Danger helps us a lot especially for a big OSS. Maybe it's time to introduce it!

@cojoj
Copy link
Contributor Author

cojoj commented Jun 7, 2017

It's a nice addition for keeping consistency in project. Especially when it comes to modifying more than just a source files. Right now you have to remember everything by heart... If you add something, update README etc.

@JohnSundell
Copy link
Owner

Sounds great! I'd love to add Danger to Marathon. I'll update the issue title to "Add Danger" and set it to ready for implementation. Anyone up for the task? 😄

@JohnSundell JohnSundell changed the title Report linting warnings to PR Add Danger Jun 7, 2017
@cojoj
Copy link
Contributor Author

cojoj commented Jun 7, 2017

I can play with it @JohnSundell. I always wanted to give Danger a spin 😎

@JohnSundell
Copy link
Owner

@cojoj Awesome 🚀

@cojoj
Copy link
Contributor Author

cojoj commented Jun 8, 2017

I'm working on it in #106 🙌 Feel free to write what you;d like to see in Dangefile

@cojoj
Copy link
Contributor Author

cojoj commented Jun 8, 2017

It's super strange that I can't make SwiftLint work with our CI... It's not reporting at all! I don't know what's the problem... I've tried looking up some similar set ups and for example firefox has it and it works just fine for them! It's like swiftlint.lint_files isn't called at all 😱
@pixyzehn have you ever faced such issue? I don't know if it's a bug or maybe I'm missing something in integration...

@cojoj cojoj mentioned this issue Jun 8, 2017
@pixyzehn
Copy link
Contributor

pixyzehn commented Jun 9, 2017

@cojoj I have never faced that...🤔

@cojoj
Copy link
Contributor Author

cojoj commented Jun 9, 2017

@pixyzehn FYI solved it... Looks like our .swiftlint.yml had some conflicts. If you're interested you can take a look at: https://github.com/ashfurrow/danger-swiftlint/issues/43

Also, please take a look and verify .swiftlint.yml in #106

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

No branches or pull requests

3 participants