openerp-community-reviewer team mailing list archive

["Guewen Baconnier @ Camptocamp" <guewen.baconnier@xxxxxxxxxxxxxx>]

Guewen Baconnier @ Camptocamp has proposed merging lp:~camptocamp/lp-community-utils/openerp-reviewers-nag-votes into lp:lp-community-utils.

Commit message:
[ADD] summary of votes for each proposal and indication if the proposal can be merged

Requested reviews:
  OpenERP Community Reviewer/Maintainer (openerp-community-reviewer)

For more details, see:
https://code.launchpad.net/~camptocamp/lp-community-utils/openerp-reviewers-nag-votes/+merge/207457

Improve the nag script in 2 ways:

 - A summary of the current votes is displayed on each lines.  I'm open to any opinions about the characters to use.
 - A new action is now displayed when a merge can be merged according to the community rules (5 days and 2 reviewers or less days but 3 reviewers). The action will no display "A committer should consider to merge the proposal".

I observed that the votes cannot be retrieved from the Launchpad's API if the script is launched as an anonymous user. It should be used with --authenticated. I wonder if it should become the default.

Now an example:

$ ./openerp-nag -p sale-wkfl --authenticated 
================================================================================
                       Done thinking, here's the nag list                       
================================================================================
 1: [age 84] (votes 2+) A committer should consider to merge the proposal https://code.launchpad.net/~camptocamp/sale-wkfl/7.0-add-sale_stock_global_delivery_lead_time-afe/+merge/197205 on the project sale-wkfl
 2: [age 84] (votes 4+) A committer should consider to merge the proposal https://code.launchpad.net/~camptocamp/sale-wkfl/7.0-add_reorderable_lines-afe/+merge/197175 on the project sale-wkfl
 3: [age 69] (votes 1?, 1!) Someone should review the merge request https://code.launchpad.net/~camptocamp/sale-wkfl/add-jit-module-for-service-product-jge/+merge/198960 on the project sale-wkfl
 4: [age 43] (votes 1*) Someone should review the merge request https://code.launchpad.net/~akretion-team/sale-wkfl/7.0-add-sale_group/+merge/201017 on the project sale-wkfl
 5: [age 42] (votes 1*) Someone should review the merge request https://code.launchpad.net/~akretion-team/sale-wkfl/7.0-add-sale_import_lines/+merge/201021 on the project sale-wkfl
 6: [age 37] (votes 1*) Someone should review the merge request https://code.launchpad.net/~akretion-team/sale-wkfl/70-sale-validity-enhanced/+merge/201762 on the project sale-wkfl
 7: [age 31] (votes 1+, 1*) Someone should review the merge request https://code.launchpad.net/~agilebg/sale-wkfl/adding_product_customer_code_sale_7/+merge/202468 on the project sale-wkfl
 8: [age 29] (votes 1!, 1*) Someone should review the merge request https://code.launchpad.net/~akretion-team/sale-wkfl/70-add-sale_order_revision/+merge/202735 on the project sale-wkfl
 9: [age 10] (votes 1*) Someone should review the merge request https://code.launchpad.net/~david-cormier-j/sale-wkfl/sale_landed_costs/+merge/205650 on the project sale-wkfl
10: [age 10] (votes 1+) Someone should review the merge request https://code.launchpad.net/~camptocamp/sale-wkfl/7.0-avoid_sale_lines_dropshipping_make_to_order_without_supplierinfos-rde/+merge/205623 on the project sale-wkfl
Votes legend:
? Needs Information
~ Abstain
- Disapprove
! Needs Fixing
R Resubmit
+ Approve
* Pending

-- 
https://code.launchpad.net/~camptocamp/lp-community-utils/openerp-reviewers-nag-votes/+merge/207457
Your team OpenERP Community Reviewer/Maintainer is requested to review the proposed merge of lp:~camptocamp/lp-community-utils/openerp-reviewers-nag-votes into lp:lp-community-utils.

=== modified file 'openerp-nag'
--- openerp-nag	2013-09-22 14:36:16 +0000
+++ openerp-nag	2014-02-20 14:59:50 +0000
@@ -57,7 +57,7 @@
 # ...with the specified 'action'
 # ...about a particular 'subject'
 Nag = collections.namedtuple(
-    "Nag", "person action subject sort_class sort_priority sort_age project_name")
+    "Nag", "person action subject sort_class sort_priority sort_age project_name votes")
 
 SORT_CLASS_BUG, SORT_CLASS_MERGE = range(2)
 
@@ -83,6 +83,15 @@
 
     max_bug_new_age = 7
 
+    # minimal number of reviewers before merging
+    min_approve = 2
+
+    # can't be merged before n days
+    days_before_merge = 5
+
+    # can be merged before days_before_merge if at least n approvals
+    approvals_to_bypass = 3
+
 
 class UTC(datetime.tzinfo):
 
@@ -96,6 +105,62 @@
 UTC = UTC()
 
 
+class Votes(object):
+    """ Holds votes of a proposal """
+
+    __states__ = {
+        'Approve': 'approve',
+        'Needs Fixing': 'needs_fixing',
+        'Needs Information': 'needs_information',
+        'Abstain': 'abstain',
+        'Disapprove': 'disapprove',
+        'Resubmit': 'resubmit',
+        'Pending': 'pending',
+    }
+
+    __signs__ = {
+        'approve': '+',
+        'needs_fixing': '!',
+        'needs_information': '?',
+        'abstain': '~',
+        'disapprove': '-',
+        'resubmit': 'R',
+        'pending': '*',
+    }
+
+    __ignore_for_approval__ = ['abstain', 'pending']
+
+    def __init__(self, votes):
+        self._votes = collections.Counter(
+            self.__states__[vote.comment.vote if vote.comment else 'Pending']
+            for vote in votes
+        )  # no comment is a pending review, usually the team
+
+    def __getattr__(self, name):
+        if name in self._votes:
+            return self._votes[name]
+        elif name in self.__states__.values():
+            return 0
+        else:
+            raise AttributeError
+
+    def __str__(self):
+        signs = ['%d%s' % (count, self.__signs__[state]) for
+                 state, count in self._votes.iteritems()]
+        return ', '.join(signs)
+
+    def total(self, for_approval=False):
+        if for_approval:
+            return sum(count for state, count in self._votes.iteritems()
+                       if state not in self.__ignore_for_approval__)
+        return sum(self._votes.values())
+
+    @classmethod
+    def legend(cls):
+        return dict((state, cls.__signs__[code]) for
+                    state, code in cls.__states__.iteritems())
+
+
 def gen_project_nags(lp, policy, project_name):
     # TODO: Detect project groups and redirect the nag to all projects
     # underneath that project
@@ -126,6 +191,8 @@
         # Skip everything that is already merged
         if proposal.date_merged is not None:
             continue
+        votes = Votes(proposal.votes)
+
         # Nag about missing commit message on merge requests
         if policy.mps_need_commit_message and proposal.commit_message is None:
             yield Nag(
@@ -135,10 +202,30 @@
                 sort_class=SORT_CLASS_MERGE,
                 sort_priority=None,  # TODO: get from max(linked bugs)
                 sort_age=(proposal.date_review_requested - now).days,
-                project_name=project.name
-            )
+                project_name=project.name,
+                votes=votes,
+            )
+
+        age = (now - proposal.date_review_requested).days
+
+        if (votes.approve == votes.total(for_approval=True) and
+                (votes.approve >= policy.approvals_to_bypass or
+                 votes.approve >= policy.min_approve and
+                 age >= policy.days_before_merge)):
+            yield Nag(
+                person='A committer',
+                action="consider to merge the proposal",
+                subject=proposal.web_link,
+                sort_class=SORT_CLASS_MERGE,
+                sort_priority=None,  # TODO: get from max(linked bugs)
+                sort_age=(proposal.date_review_requested - now).days,
+                project_name=project.name,
+                votes=votes,
+            )
+            continue
+
         # Nag about aging merge requests
-        if (now - proposal.date_review_requested).days >= policy.max_review_age:
+        if age >= policy.max_review_age:
             yield Nag(
                 person='Someone',
                 action="review the merge request",
@@ -146,7 +233,8 @@
                 sort_class=SORT_CLASS_MERGE,
                 sort_priority=None,  # TODO: get from max(linked bugs)
                 sort_age=(proposal.date_review_requested - now).days,
-                project_name=project.name
+                project_name=project.name,
+                votes=votes,
             )
 
 
@@ -164,7 +252,8 @@
             subject=bug_task.web_link,
             sort_class=SORT_CLASS_BUG,
             sort_priority=None,  # TODO: convert from importance name
-            sort_age=None)
+            sort_age=None,
+            votes=None)
 
 
 def parse_projects_file(filename):
@@ -225,6 +314,9 @@
     if args.projects_file:
         project_names.extend(parse_projects_file(args.projects_file))
 
+    if args.anonymous:
+        print('Warning: it is advised to use the --authenticated argument.')
+
     # Find things to nag about
     nags = []
     for project_name in project_names:
@@ -236,11 +328,19 @@
     print("Done thinking, here's the nag list".center(80))
     print("=" * 80)
     for index1, nag in enumerate(nags, 1):
-        print("{index1:-2}: [age {age}] {person} should {action} {subject} "
+        print("{index1:-2}: [age {age}] (votes {votes}) {person} should {action} {subject} "
               "on the project {project}".format(
                   index1=index1, age=(nag.sort_age and -nag.sort_age), person=nag.person,
-                  action=nag.action, subject=nag.subject, project=nag.project_name))
+                  action=nag.action, subject=nag.subject, project=nag.project_name,
+                  votes=nag.votes or ''))
 
+    print('Votes legend:')
+    if args.anonymous:
+        print('No votes can be displayed as anonymous user. '
+              'Use the --authenticated argument to see the votes.')
+    else:
+        for state, sign in Votes.legend().iteritems():
+            print('%s %s' % (sign, state))
 
 if __name__ == "__main__":
     try: