Approval category NoBlock still flakey on 2.5

67 views
Skip to first unread message

Phil Hord

unread,
Nov 15, 2012, 2:01:00 PM11/15/12
to repo-d...@googlegroups.com
I may have some things wrong here because it feels like most of this stuff is un-/poorly-documented.  But I am trying to add a new approval category to our Gerrit instance.  Here's what I did:

  insert into approval_categories(name, abbreviated_name, position, function_name, copy_min_score, category_id) VALUES('Flags', 'F', 2, 'NoBlock', 'N', 'FLAG');
  insert into approval_category_values(name, category_id, value) VALUES('No flags', 'FLAG', 0);
  insert into approval_category_values(name, category_id, value) VALUES('Flag: revisit for cleanup', 'FLAG', 1);

It seems to be working now. 
  I can add the permission. 
  I can see the 'Flags' column on changes. 
  I can set a flag on a change in review. 
  I can see the 'F' column on the change list.
  I can search for 'Flags+1' and find the correct hits.

However:
  The 'F' column is empty on the changes screen.
  The "Flags+1" approval does not show up in the refs/notes/review

Is this because it is not needed for submit? 

Will the display column always be empty?  If so, shouldn't it be omitted?

I really wanted this to be a [-1..0] range, but I couldn't get Gerrit to recognize the new category unless it had a positive possible value defined.  Is that expected?

Phil

p.s. I had to restart Gerrit several times before the new "Label Flags" permission showed up.  Before one of those reboots, I changed it to function_name='MaxWithBlock', and later I changed it back to function_name='NoBlock'.  Changing it to MaxWithBlock seems to be what helped me adding the "Label" permission, but I cannot verify that for sure.  Here is where I ended up:

Welcome to Gerrit Code Review 2.5-1
(PostgreSQL 8.4.8)

Type '\h' for help.  Type '\r' to clear the buffer.

gerrit> select *  from approval_categories;
 name        | abbreviated_name | position | function_name | copy_min_score | category_id
 ------------+------------------+----------+---------------+----------------+------------
 Verified    | V                | 0        | MaxWithBlock  | N              | VRIF
 Code Review | R                | 1        | MaxWithBlock  | Y              | CRVW
 Flags       | F                | 2        | NoBlock       | N              | FLAG
(3 rows; 6 ms)
gerrit> select * from  approval_category_values ;
 name                                            | category_id | value
 ------------------------------------------------+-------------+------
 Administer All Settings                         | OWN         | 1
 Upload permission                               | READ        | 2
 Read access                                     | READ        | 1
 No access                                       | READ        | -1
 Verified                                        | VRIF        | 1
 No score                                        | VRIF        | 0
 Fails                                           | VRIF        | -1
 Looks good to me, approved                      | CRVW        | 2
 Looks good to me, but someone else must approve | CRVW        | 1
 No score                                        | CRVW        | 0
 I would prefer that you didn't submit this      | CRVW        | -1
 Do not submit                                   | CRVW        | -2
 Submit                                          | SUBM        | 1
 Create Signed Tag                               | pTAG        | 1
 Create Annotated Tag                            | pTAG        | 2
 Update Branch                                   | pHD         | 1
 Create Branch                                   | pHD         | 2
 Force Push Branch; Delete Branch                | pHD         | 3
 Forge Author Identity                           | FORG        | 1
 Forge Committer or Tagger Identity              | FORG        | 2
 Forge Gerrit Code Review Server Identity        | FORG        | 3
 Upload merges permission                        | READ        | 3
 No flags                                        | FLAG        | 0
 Flag: revisit for cleanup                       | FLAG        | 1
 Flag: unused-flag-for-testing                   | FLAG        | -1
(25 rows; 7 ms)



Saša Živkov

unread,
Nov 15, 2012, 4:10:19 PM11/15/12
to Phil Hord, repo-d...@googlegroups.com
On Thu, Nov 15, 2012 at 8:01 PM, Phil Hord <ho...@cisco.com> wrote:
I may have some things wrong here because it feels like most of this stuff is un-/poorly-documented.  But I am trying to add a new approval category to our Gerrit instance.  Here's what I did:

  insert into approval_categories(name, abbreviated_name, position, function_name, copy_min_score, category_id) VALUES('Flags', 'F', 2, 'NoBlock', 'N', 'FLAG');
  insert into approval_category_values(name, category_id, value) VALUES('No flags', 'FLAG', 0);
  insert into approval_category_values(name, category_id, value) VALUES('Flag: revisit for cleanup', 'FLAG', 1);

It seems to be working now. 
  I can add the permission. 
  I can see the 'Flags' column on changes. 
  I can set a flag on a change in review. 
  I can see the 'F' column on the change list.
  I can search for 'Flags+1' and find the correct hits.

However:
  The 'F' column is empty on the changes screen.
this is strange. What do you find in the patchset_approvals table for a patch set
which has a vote in the Flags category.

  The "Flags+1" approval does not show up in the refs/notes/review
this is probably a consequence of the previous issue.
 

Is this because it is not needed for submit? 
As I see from the code only votes with value zero will be skipped when creating
a note in the refs/notes/review branch.
 

Phil Hord

unread,
Nov 15, 2012, 4:56:37 PM11/15/12
to Saša Živkov, repo-d...@googlegroups.com

Sa�a �ivkov wrote:
> On Thu, Nov 15, 2012 at 8:01 PM, Phil Hord <ho...@cisco.com> wrote:
>
>> I may have some things wrong here because it feels like most of this stuff
>> is un-/poorly-documented. But I am trying to add a new approval category
>> to our Gerrit instance. Here's what I did:
>>
>> insert into approval_categories(name, abbreviated_name, position,
>> function_name, copy_min_score, category_id) VALUES('Flags', 'F', 2,
>> 'NoBlock', 'N', 'FLAG');
>> insert into approval_category_values(name, category_id, value)
>> VALUES('No flags', 'FLAG', 0);
>> insert into approval_category_values(name, category_id, value)
>> VALUES('Flag: revisit for cleanup', 'FLAG', 1);
>>
>> It seems to be working now.
>> I can add the permission.
>> I can see the 'Flags' column on changes.
>> I can set a flag on a change in review.
>> I can see the 'F' column on the change list.
>> I can search for 'Flags+1' and find the correct hits.
>>
>> However:
>> The 'F' column is empty on the changes screen.
>>
> this is strange. What do you find in the patchset_approvals table for a
> patch set
> which has a vote in the Flags category.

value | granted | change_open | change_sort_key |
change_id | patch_set_id | account_id | category_id
------+----------------------------+-------------+------------------+-----------+--------------+------------+------------
-2 | 2012-11-12 14:22:56.861-05 | Y | 00211b4300000c3d |
3133 | 1 | 1000002 | CRVW
1 | 2012-11-15 12:07:22.764-05 | Y | 00211b4300000c3d |
3133 | 1 | 1000002 | FLAG
0 | 2012-11-12 14:22:56.861-05 | Y | 00211b4300000c3d |
3133 | 1 | 1000002 | VRIF
0 | 2012-11-12 14:20:34.177-05 | Y | 00211b4300000c3d |
3133 | 1 | 1000011 | CRVW
(4 rows; 2 ms)


Here's the result of the JSON query for "Flags+1":

$ ssh gerrit gerrit query --current-patch-set --format=JSON "Flags+1"
{"project":"core","branch":"3.0","id":"If015d35cd8f23ff175b2b3b938cf98ddd55e7ebd","number":"3133","subject":"Update
this project for 3.0","owner":{"name":"Phil
Hord","email":"ho...@cisco.com","username":"hordp"},"url":"https://grt.cisco.com/gerrit/3133","createdOn":1352747668,"lastUpdated":1352999242,"sortKey":"00211b4300000c3d","open":true,"status":"NEW","currentPatchSet":{"number":"1","revision":"527b0f1f8676d2573b79556a012a5836a729fb02","parents":["4d2d19f8e2f5487a6e72c0a342a1af7fe26d653f"],"ref":"refs/changes/33/3133/1","uploader":{"name":"Phil
Hord","email":"ho...@cisco.com","username":"hordp"},"createdOn":1352747668,"approvals":[{"type":"CRVW","description":"Code
Review","value":"-2","grantedOn":1352748176,"by":{"name":"Phil
Hord","email":"ho...@cisco.com","username":"hordp"}},{"type":"FLAG","description":"Flags","value":"1","grantedOn":1352999242,"by":{"name":"Phil
Hord","email":"ho...@cisco.com","username":"hordp"}}]}}
{"type":"stats","rowCount":1,"runTimeMilliseconds":155}

It shows the FLAG value is 1, but it still does not show this in the web
page.



> The "Flags+1" approval does not show up in the refs/notes/review
> this is probably a consequence of the previous issue.

Or it may be that the patch was rebased just before it was merged so my
flag was lost. I'll try to recreate that issue on another commit.

Phil Hord

unread,
Nov 15, 2012, 5:06:05 PM11/15/12
to Saša Živkov, repo-d...@googlegroups.com
It was the rebase. I tried it again just now on a single patchset and
it did show the flags in the notes. I apologize for the misdirection.

But I do still have the web page mystery of the empty "F" column on my
Flagged, unmerged changes.

I'm also a little disappointed now since I was hoping to have these
flags to go back to later, but they are easily lost during review
cycles. I'll have to settle for gleaning them from the database
instead. Maybe the approval_categories table needs a "copy_max_scores"
flag, or maybe I need to make my flag a -1.


Phil

Reply all
Reply to author
Forward
0 new messages