Branch status notification via Gitter #413
Conversation
europ
force-pushed
the
Gitter
branch
9 times, most recently
from
ab7a585 to
0f67274
Compare
europ
changed the title
|
@europ what branch status do you want to broadcast? If it is the travis build status, please update the PR description and the documentation accordingly. |
app/workers/gitter_notificator.rb
Outdated
| @gitter_token = Settings.travis_monitor.gitter_token | ||
|
|
||
| Settings.travis_monitor.branches.each do |element| | ||
| check(element.repo, element.branch, element.gitter_room) |
There was a problem hiding this comment.
I think this check method does too many things at once. Let's take these functionalities apart:
- A method that checks the build change status
- A method that builds a message based on the status
- A method that sends the message to gitter
app/workers/gitter_notificator.rb
Outdated
| gitter_send(room, msg) | ||
| end | ||
|
|
||
| def latest_builds(repository, branch, count) |
There was a problem hiding this comment.
Do you really need the count attribute? I don't think this method needs to be so universal.
app/workers/gitter_notificator.rb
Outdated
| end | ||
|
|
||
| def latest_builds(repository, branch, count) | ||
| latest_builds = [] |
There was a problem hiding this comment.
You don't need to declare a helper array if you use Enumerable#take_while and then Enumerable#select or something similar from here.
| let(:gitter_rooms) { [double("Room", :name => "wrong1", :id => 1), double("Room", :name => room, :id => 2), double("Room", :name => "wrong2", :id => 3)] } | ||
| let(:gitter_client) { double("Gitter::Client", :rooms => gitter_rooms) } | ||
|
|
||
| it "should send a message" do |
There was a problem hiding this comment.
|
|
||
| it "should call check with proper parameters" do | ||
| allow(item_travis_monitor).to receive(:branches).and_return([item_branch]) | ||
| allow(Settings).to receive(:travis_monitor).and_return(item_travis_monitor) |
There was a problem hiding this comment.
I see similar mocks in multiple its, can you please pull them out into a before block?
| subject.send(:init) | ||
| end | ||
|
|
||
| it "should call check 3 times" do |
|
|
||
| describe "#check" do | ||
| it "should send a broken branch message" do | ||
| allow(subject).to receive(:latest_builds).and_return(%w(passed failed)) |
There was a problem hiding this comment.
Common mocks to a before block please:
before do
allow(rick).to receive(:mega_seed).and_return(intelligence)
end
context "intelligence is exceptional" do
let(:intelligence) { :exceptional_intelligence }
it ...
end
context "intelligence is unquestionable" do
let(:intelligence) { :unquestionable_intelligence }
it ...
end| let(:invalid_unexpected_build) { double("Build", :state => "state", :branch_info => "wrong branch") } | ||
|
|
||
| let(:travis_builds) { [first_expected_build, invalid_unexpected_build, second_expected_build, valid_unexpected_build] } | ||
| let(:travis_repository) { double("Travis::Repository", :builds => travis_builds) } |
There was a problem hiding this comment.
Why do you need the travis_build variable? Use just a single multiline let block.
|
|
||
| describe "#gitter_send" do | ||
| let(:message) { "hello" } | ||
| let(:@gitter_token) { "token" } |
|
We already have this with the travis integration in Gitter...it shows up in the right side panel. FWIW, a lot of people refuse to use the IRC bridge to Gitter because those notifications flood the room and it becomes unreadable, so I've been considering disabling them. |
europ
force-pushed
the
Gitter
branch
4 times, most recently
from
3759c8a to
eb352c8
Compare
Added new worker which checks the latest two builds on a specified branch in a specified repository and send a notifying message to the specified gitter room. This message inform the room members about the status of the branch - branch is broken or fixed. The message depends on the latest two builds from which is the report created. Required specifications must be included in the file containing the settings.
|
Checked commit europ@416e425 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 **
|
|
I'm on the fence about this feature. Likes:
Dislikes:
If we were to trim down the side panel to just branch changes (and only for specific branches), then I think we accomplish the same things as this PR without having to maintain any code. At that point it might be useful to have the direct notifications in-channel. |
|
This pull request is not mergeable. Please rebase and repush. |
|
@NickLaMuro started to resurrect this via #452 , so I'm going to close. |
A new feature "notify gitter room members about repository branch fix/break" was added. It is performed via gem ruby-gitter.
A new worker is checking the state of latest two builds from which is stated the action for notification. Every 10 minutes worker gets the latest two builds of a specified branch in a specified repository from which is the state of the build obtained. The state of these builds decides about sending a message to a specified room on gitter. The judgement about broken branch is created when the states are [failed, passed] (first element of the array is the latest build state). The judgement about fixed branch is created when the states are [passed, failed]. Otherwise, when the states are [passed, passed] or [failed, failed] there is no action performed. The required specification (gitter token, repository, branch, gitter room) must be included in the file containing the settings.
Gitter notification message:
broken branch:
🆘⚠️ "branch" in "owner/repository" is broken ‼️ 💥
fixed branch:
✅ Broken branch has been fixed 💚
\cc
@skateman