-
Notifications
You must be signed in to change notification settings - Fork 245
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
Add GitHub status formatter #144
Add GitHub status formatter #144
Conversation
module Formatter | ||
class GithubStatusFormatter | ||
def initialize(opts = {}) | ||
@level_mapping ||= opts.fetch(:level_mapping, {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if it's worth having a way to override it. Let's leave it off for now 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for implementing this was that Reek was returning only warning
s even though stuff like Doesn't depend on instance state (maybe move it to another class?) (UtilityFunction)
sound pretty subjective to me. Now when I think about it, it'd probably be better to address the issue at mmozuras/pronto-reek than here.
I'll get rid of opts
parameter for now.
@mknapik amazing, thank you, honestly 👏 😄 Just address a couple of comments, add info to README and CHANGELOG and it's gonna get merged! 😄 |
The new formatter allows to submit statuses to GitHub commits.
@mmozuras Apart from adhering to your remarks I've also did some refactoring. |
Use `GithubStatusFormatter` to submit [commit status](https://github.com/blog/1227-commit-status-api): | ||
|
||
```sh | ||
$ GITHUB_ACCESS_TOKEN=token pronto run -f github_status -c origin/master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that it's worth showing that multiple formatters can be used here too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'll add one more example.
Although I believe using multiple formatters is not documented anywhere else.
@mknapik looks good, just address my one last comment 😄 |
@aergonaut Actually I had no idea what was the convention. |
@mmozuras @aergonaut Regarding context names: The difference:
|
@mknapik looks good to me 😄 |
@mknapik looks good, amazing contribution 👍 🙇 |
@mmozuras Thanks! |
This is awesome. Was trying to make it work using version 0.6.0, then realized that this isn't released yet 😉 . I look forward to using it in the next version bump. |
Solves #121.
The formatter creates one commit status per runner with information about issue counts.
Issues in status description are ordered by severity.
By default all Pronto Messages with level
:warning
or higher are considered a failed state for GitHub status (red status).GithubStatusFormatter
allows to override this behaviour by accepting:level_mapping
option in the initializer (which is not supported when running the formatter directly with command line, e.g.pronto run -f github_pr_status
).There is no README info yet but I'll provide it once we agree on the interface and the way this formatter should work.
The result of running Pronto with the new formatter:
![GithubStatusFormatter](https://cloud.githubusercontent.com/assets/249141/14524059/e4831ef4-0236-11e6-96be-a652761e354a.png)
@mmozuras Please let me know whether the PR adheres to the project conventions.
I got some RuboCop
line to long warnings
in specs, should I fix those?Also I'm going to squash refactoring commits before merging.