Refactor into Reporter Interface #113
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously the github.Analyse test was responsible for testing that the Status API states were being set in the correct order. This complicated the testing and we'll shortly be duplicating some of that functionality and tests into its own type. In moving to its own type, we won't be able to track the order of statuses, we want to see Pending be set first, followed by Success. The unit tests will test to make sure the status API can set the status correctly, but won't have visibility in the order they occur. Instead, the integration tests, which are already checking the status API statuses will be responsible to ensure the state transitions from Pending to Success/Failure or Error.
![gopherci[bot]](https://avatars.githubusercontent.com/in/889?s=60&v=4){kind=small}
internal/github/reporters.go
Outdated
| type StatusState string | ||
|
|
||
| const ( | ||
| StatusStatePending StatusState = "pending" |
There was a problem hiding this comment.
golint: exported const StatusStatePending should have comment (or a comment on this block) or be unexported
internal/github/reporters.go
Outdated
| StatusStateFailure StatusState = "failure" | ||
| ) | ||
|
|
||
| type StatusAPIReporter struct { |
There was a problem hiding this comment.
golint: exported type StatusAPIReporter should have comment or be unexported
internal/github/reporters.go
Outdated
|
|
||
| var _ analyser.Reporter = &StatusAPIReporter{} | ||
|
|
||
| func NewStatusAPIReporter(client *github.Client, statusURL, context, targetURL string) *StatusAPIReporter { |
There was a problem hiding this comment.
golint: exported function NewStatusAPIReporter should have comment or be unexported
internal/github/reporters_test.go
Outdated
| } | ||
| } | ||
|
|
||
| func PRCommentReporter_report(t *testing.T) { |
There was a problem hiding this comment.
golint: don't use underscores in Go names; func PRCommentReporter_report should be PRCommentReporterReport
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously the GitHub handler was solely responsible for reporting issues, but in the near future, the Analyser package will begin to be responsible for it.
To do this, we'll create an
analyser.Reporterinterface with a single methodReport(context.Context, []db.Issue) error. GitHub handler is then responsible for determining which reporters it requires and then executing those reporters.GitHub handler will still be responsible for calling the Report methods, but this will eventually change, and instead, GitHub handler will just be responsible for instantiating the types and passing them to Analyser.
This change is mostly moving the two reporters (Pull Request Comments and Status API) to their own types that implement the
analyser.Reporterinterface.Additional reporters will be available in #75 (commit comments) and #112 (pull request reviews).
Included: Integration tests to test for states being set in order