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.
I'm not 100% happy with this change. It definitely simplifies the creation of slack messages from the data that is generated by the TeamCity build status events. However, I notice that the
SlackWrapper
class is basically a useless abstraction in the current way the code is organized.For each event the
SlackNotificator
receives and each user in the set of users who need to be notified for this event, we instantiate a newSlackWrapper
and then callsend
on that wrapper instance. But, we never need to reuse any of the state on theSlackWrapper
object, so, we might as well just be passing the data straight through instead of storing it on an object.I'd like to get this much of the code integrated into the project as it provides some benefit (no longer need the
SlackPayload
class.) But, then I'm going to continue restructuring theSlackNotificator
such that it's job is basically to:TeamCityEvent
object which describes what happened from TeamCity's point of view (e.g. The build failed.)SlackTarget
object which describes the data we currently keep onSlackWrapper
and then passes thatSet<SlackTarget>
andTeamCityEvent
to a new version ofSlackWrapper.send()
which will just be responsible for creating aSlackMessage
and sending it to theSlackApi
for eachSlackTarget
andTeamCityEvent
This way, we can construct theSlackWrapper
only once and cache instances ofSlackApi
based on theslackUrl
from which they are constructed.If you'd prefer, I can go ahead and work on those changes as part of this pull request, but this code is working, so I'd prefer to get it checked in, and then add these other pieces incrementally.