My favorites | Sign in
Project Home Downloads Wiki Issues Source
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 2465: Dedicated notification channel for patch set verifications
4 people starred this issue and may be notified of changes. Back to list
Status:  ChangeUnderReview
Owner:  ----


Sign in to add a comment
 
Project Member Reported by David.Os...@gmail.com, Feb 11, 2014
Problem:
======

Currently the patch set verification notifications use the same channel: gerrit review command and patch_set_approvals table to publish not only actually patch set verification votes, but also the build
status changes:

* Build started on `date`: no Vote
* Build was successful: with or without Vote

Only simple projects have single job verification per project. In reality the verification is separated to
multiple jobs, where you are ending up with:

* Build on Platform `foo` started on date: no Vote
* Build on Platform `foo` was successful on date: no Vote, because who knows how other jobs are doing?
[…] other n-1 platforms/jobs notifications
* Combined vote: all n platforms did well: with Vote VRFY: "Verified"

So for one patch set verification you are going to spam 2n+1 times the reviewer comment table. Now new patch set i upload.
How valuable these information or better to say spam is now?  It is completely useless because new patch
set means: new game, new luck.

Solution:
======

Shut down verification firehose and stop spamming reviewer table and provide own dedicated channel
for verification notifications.
Human reviews and bot notifications have nothing in common:

* [human votes] from previous patch sets are valuable
* [human votes] only need text and/or image link
* [bots notification] from previous patch sets a re totally useless (but they can not be hidden)
* [bots notification] have very important additional info: link to the CI log
* [human votes] location in comment reviewer table works very well
* [bots notification] scattered across and between human reviews horribly sucks

Instead of putting bots notifications in comment table, it should be in more prominent place:
on the top of CS2, and be presented in its own table:

Job/Category | started on | ended on | href | status
foo [...]
bar [...]
bat […]

One of the most important improvements with introduction of new change screen is the ability to switch
the context of presented information on patch set switch in Revisions drop down:

It works fine with the own verification table concept:
The content of the verification table should be reloaded on patch set switch, so that the status of jobs
outcome correspond to the patch set (currently scrolling and searching in reviewer table is needed).
No spam of reviewer table any more.

Implementation:
==========

in core Approach
--------------
 
* Client: render new table on patch set switch
* REST: add new endpoints
* Persistence: it doesn't really make sense to introduce new table patch_set_verifications,
during the work on evacuation of the ReviewDB is ongoing. The better approach would be probably to try to store it in the NotesDB.
* SSH command: add new command to publish notifications with additional info

Plugin approach
--------------

Almost the same as in core
JS-API allows us already to put custom fragments on change screen 2.
Persistence problem must be solved, though.

Mixed approach
--------------

Have parts in core (persistence ?) and have friendly different plugins that implement/use it,
a lá download-commands and avatar plugins.

Jul 6, 2014
Project Member #1 David.Os...@gmail.com
https://gerrit-review.googlesource.com/58283
Status: ChangeUnderReview
Labels: -Priority-Minor Priority-Major
Sign in to add a comment

Powered by Google Project Hosting