Patchwork [2,of,2] releasenotes: add similarity check function to compare incoming notes

login
register
mail settings
Submitter Rishabh Madan
Date July 3, 2017, 9:06 p.m.
Message ID <1fc4f41fadcca56d36ed.1499115991@bunty>
Download mbox | patch
Permalink /patch/21958/
State Superseded
Headers show

Comments

Rishabh Madan - July 3, 2017, 9:06 p.m.
# HG changeset patch
# User Rishabh Madan <rishabhmadan96@gmail.com>
# Date 1499115687 -7200
#      Mon Jul 03 23:01:27 2017 +0200
# Node ID 1fc4f41fadcca56d36edd0747a09e1be6af5f680
# Parent  6684514628b44e838f499b2125ffebe8d700a204
releasenotes: add similarity check function to compare incoming notes

It is possible that the incoming note fragments might have some similar content
as the existing release notes. In case of a bug fix, we match for issueNNNN in $
existing notes. For other general cases, it makes use of fuzzywuzzy library to
get a similarity score. If the score is above a certain threshold, we ignore
the fragment otherwise add it. But the score might be misleading for small comm$
messages. So, it uses similarity function if only the length of string (in word$
is above a certain number. The patch also adds tests related to its usage.
But it needs improvement in the sense of combining the incoming notes. We can
use interactive mode for adding the notes. Maybe we can do this if similarity
score is under a certain range.
Yuya Nishihara - July 5, 2017, 1:44 p.m.
On Mon, 03 Jul 2017 23:06:31 +0200, Rishabh Madan wrote:
> # HG changeset patch
> # User Rishabh Madan <rishabhmadan96@gmail.com>
> # Date 1499115687 -7200
> #      Mon Jul 03 23:01:27 2017 +0200
> # Node ID 1fc4f41fadcca56d36edd0747a09e1be6af5f680
> # Parent  6684514628b44e838f499b2125ffebe8d700a204
> releasenotes: add similarity check function to compare incoming notes

Can you think more about how to split testable functions?

> @@ -89,7 +91,20 @@
>  
>          This is used to combine multiple sources of release notes together.
>          """
> +
> +        all_points = []
> +
>          for section in other:
> +            for title, paragraphs in self.titledforsection(section):
> +                str = ""
> +                str = converttostring(paragraphs)
> +                all_points.append(str)
> +
> +            for paragraphs in self.nontitledforsection(section):
> +                str = ""
> +                str = converttostring(paragraphs)
> +                all_points.append(str)

 (*1)

> @@ -98,17 +113,58 @@
>                               (title, section))
>                      continue
>  
> -                # TODO perform similarity comparison and try to match against
> -                # existing.
> -                self.addtitleditem(section, title, paragraphs)
> +                str_incoming = converttostring(paragraphs)
> +                if section == 'fix':
> +                    issues = re.findall(RE_ISSUE, str_incoming, re.IGNORECASE)
> +                    if len(issues) > 0:

Nit: If only first match is needed, re.search() should be enough.

> +                        issuenumber = issues[0]
> +                        issuenumber = "".join(issuenumber.split())
> +                        if any(issuenumber in s for s in all_points):
> +                            ui.write(_("\"%s\" already exists in notes; "
> +                                     "ignoring\n") % issuenumber)
> +                            continue
> +                        else:
> +                            self.addtitleditem(section, title, paragraphs)
> +                            continue
> +
> +                if len(str_incoming.split()) > 10:
> +                    merge = similaritycheck(str_incoming, all_points)
> +
> +                    if not merge:
> +                        ui.write(_("\"%s\" already exists in notes file; "
> +                                 "ignoring\n") % str_incoming)
> +                    else:
> +                        self.addtitleditem(section, title, paragraphs)
> +                else:
> +                    self.addtitleditem(section, title, paragraphs)
>  
>              for paragraphs in other.nontitledforsection(section):
> +                str_incoming = converttostring(paragraphs)
> +                if section == 'fix':
> +                    issues = re.findall(RE_ISSUE, str_incoming, re.IGNORECASE)
> +                    if len(issues) > 0:
> +                        issuenumber = issues[0].lower()
> +                        issuenumber = "".join(issuenumber.split())
> +                        if any(issuenumber in s for s in all_points):
> +                            ui.write(_("\"%s\" already exists in notes; "
> +                                     "ignoring\n") % str_incoming)
> +                            continue
> +                        else:
> +                            self.addnontitleditem(section, paragraphs)
> +                            continue
>                  if paragraphs in self.nontitledforsection(section):
>                      continue
>  
> -                # TODO perform similarily comparison and try to match against
> -                # existing.
> -                self.addnontitleditem(section, paragraphs)
> +                if len(str_incoming.split()) > 10:
> +                    merge = similaritycheck(str_incoming, all_points)
> +
> +                    if not merge:
> +                        ui.write(_("\"%s\" already exists in notes; "
> +                                 "ignoring\n") % str_incoming)
> +                    else:
> +                        self.addnontitleditem(section, paragraphs)
> +                else:
> +                    self.addnontitleditem(section, paragraphs)

For instance, maybe there could be a boolean function that returns whether
the new paragraph should be added or not.

  if f(section, paragraphs):
      self.addtitleditem(section, title, paragraphs)
  ...
  if f(section, paragraphs):
      self.addnontitleditem(section, paragraphs)

Since the function 'f' depends on 'all_points', which has wider scope (*1),
'f' might be a class that holds pre-computed 'all_points'.

  f = x()
  x.addknownparagraphs(paragraphs for _title, paragraphs
                       in self.titledforsection(section))
  ...

Patch

diff -r 6684514628b4 -r 1fc4f41fadcc hgext/releasenotes.py
--- a/hgext/releasenotes.py	Mon Jul 03 22:51:58 2017 +0200
+++ b/hgext/releasenotes.py	Mon Jul 03 23:01:27 2017 +0200
@@ -14,6 +14,7 @@ 
 from __future__ import absolute_import
 
 import errno
+import fuzzywuzzy.fuzz as fuzz
 import re
 import sys
 import textwrap
@@ -44,6 +45,7 @@ 
 ]
 
 RE_DIRECTIVE = re.compile('^\.\. ([a-zA-Z0-9_]+)::\s*([^$]+)?$')
+RE_ISSUE = r'\bissue [0-9]{4,6}(?![0-9])\b|\bissue[0-9]{4,6}(?![0-9])\b'
 
 BULLET_SECTION = _('Other Changes')
 
@@ -89,7 +91,20 @@ 
 
         This is used to combine multiple sources of release notes together.
         """
+
+        all_points = []
+
         for section in other:
+            for title, paragraphs in self.titledforsection(section):
+                str = ""
+                str = converttostring(paragraphs)
+                all_points.append(str)
+
+            for paragraphs in self.nontitledforsection(section):
+                str = ""
+                str = converttostring(paragraphs)
+                all_points.append(str)
+
             for title, paragraphs in other.titledforsection(section):
                 if self.hastitledinsection(section, title):
                     # TODO prompt for resolution if different and running in
@@ -98,17 +113,58 @@ 
                              (title, section))
                     continue
 
-                # TODO perform similarity comparison and try to match against
-                # existing.
-                self.addtitleditem(section, title, paragraphs)
+                str_incoming = converttostring(paragraphs)
+                if section == 'fix':
+                    issues = re.findall(RE_ISSUE, str_incoming, re.IGNORECASE)
+                    if len(issues) > 0:
+                        issuenumber = issues[0]
+                        issuenumber = "".join(issuenumber.split())
+                        if any(issuenumber in s for s in all_points):
+                            ui.write(_("\"%s\" already exists in notes; "
+                                     "ignoring\n") % issuenumber)
+                            continue
+                        else:
+                            self.addtitleditem(section, title, paragraphs)
+                            continue
+
+                if len(str_incoming.split()) > 10:
+                    merge = similaritycheck(str_incoming, all_points)
+
+                    if not merge:
+                        ui.write(_("\"%s\" already exists in notes file; "
+                                 "ignoring\n") % str_incoming)
+                    else:
+                        self.addtitleditem(section, title, paragraphs)
+                else:
+                    self.addtitleditem(section, title, paragraphs)
 
             for paragraphs in other.nontitledforsection(section):
+                str_incoming = converttostring(paragraphs)
+                if section == 'fix':
+                    issues = re.findall(RE_ISSUE, str_incoming, re.IGNORECASE)
+                    if len(issues) > 0:
+                        issuenumber = issues[0].lower()
+                        issuenumber = "".join(issuenumber.split())
+                        if any(issuenumber in s for s in all_points):
+                            ui.write(_("\"%s\" already exists in notes; "
+                                     "ignoring\n") % str_incoming)
+                            continue
+                        else:
+                            self.addnontitleditem(section, paragraphs)
+                            continue
                 if paragraphs in self.nontitledforsection(section):
                     continue
 
-                # TODO perform similarily comparison and try to match against
-                # existing.
-                self.addnontitleditem(section, paragraphs)
+                if len(str_incoming.split()) > 10:
+                    merge = similaritycheck(str_incoming, all_points)
+
+                    if not merge:
+                        ui.write(_("\"%s\" already exists in notes; "
+                                 "ignoring\n") % str_incoming)
+                    else:
+                        self.addnontitleditem(section, paragraphs)
+                else:
+                    self.addnontitleditem(section, paragraphs)
 
 class releasenotessections(object):
     def __init__(self, ui):
@@ -128,6 +184,27 @@ 
 
         return None
 
+def converttostring(paragraphs):
+    """
+    Converts paragraph and bullet data to individual strings.
+    """
+    str = ""
+    for para in paragraphs:
+        str += ' '.join(para) + ' '
+    return str
+
+def similaritycheck(incoming_str, existingnotes):
+    """
+    Returns true when note fragment can be merged to existing notes.
+    """
+    merge = True
+    for bullet in existingnotes:
+        score = fuzz.token_set_ratio(incoming_str, bullet)
+        if score > 75:
+            merge = False
+            break
+    return merge
+
 def parsenotesfromrevisions(repo, directives, revs):
     notes = parsedreleasenotes()
 
diff -r 6684514628b4 -r 1fc4f41fadcc tests/test-releasenotes-formatting.t
--- a/tests/test-releasenotes-formatting.t	Mon Jul 03 22:51:58 2017 +0200
+++ b/tests/test-releasenotes-formatting.t	Mon Jul 03 23:01:27 2017 +0200
@@ -1,3 +1,5 @@ 
+#require fuzzywuzzy
+
   $ cat >> $HGRCPATH << EOF
   > [extensions]
   > releasenotes=
diff -r 6684514628b4 -r 1fc4f41fadcc tests/test-releasenotes-merging.t
--- a/tests/test-releasenotes-merging.t	Mon Jul 03 22:51:58 2017 +0200
+++ b/tests/test-releasenotes-merging.t	Mon Jul 03 23:01:27 2017 +0200
@@ -1,3 +1,5 @@ 
+#require fuzzywuzzy
+
   $ cat >> $HGRCPATH << EOF
   > [extensions]
   > releasenotes=
@@ -158,3 +160,122 @@ 
   
   * this is fix3.
 
+  $ cd ..
+
+Ignores commit messages containing issueNNNN based on issue number.
+
+  $ hg init simple-fuzzrepo
+  $ cd simple-fuzzrepo
+  $ touch fix1
+  $ hg -q commit -A -l - << EOF
+  > commit 1
+  > 
+  > .. fix::
+  > 
+  >    Resolved issue4567.
+  > EOF
+
+  $ cat >> $TESTTMP/issue-number-notes << EOF
+  > Bug Fixes
+  > =========
+  > 
+  > * Fixed issue1234 related to XYZ.
+  > 
+  > * Fixed issue4567 related to ABC.
+  > 
+  > * Fixed issue3986 related to PQR.
+  > EOF
+
+  $ hg releasenotes -r . $TESTTMP/issue-number-notes
+  "Resolved issue4567. " already exists in notes; ignoring
+
+  $ cat $TESTTMP/issue-number-notes
+  Bug Fixes
+  =========
+  
+  * Fixed issue1234 related to XYZ.
+  
+  * Fixed issue4567 related to ABC.
+  
+  * Fixed issue3986 related to PQR.
+
+  $ cd ..
+
+Adds short commit messages (words < 10) without
+comparison unless there is an exact match.
+
+  $ hg init tempdir
+  $ cd tempdir
+  $ touch feature1
+  $ hg -q commit -A -l - << EOF
+  > commit 1
+  > 
+  > .. feature::
+  > 
+  >    Adds a new feature 1.
+  > EOF
+
+  $ hg releasenotes -r . $TESTTMP/short-sentence-notes
+
+  $ touch feature2
+  $ hg -q commit -A -l - << EOF
+  > commit 2
+  > 
+  > .. feature::
+  > 
+  >    Adds a new feature 2.
+  > EOF
+
+  $ hg releasenotes -r . $TESTTMP/short-sentence-notes
+  $ cat $TESTTMP/short-sentence-notes
+  New Features
+  ============
+  
+  * Adds a new feature 1.
+  
+  * Adds a new feature 2.
+
+  $ cd ..
+
+Ignores commit messages based on fuzzy comparison.
+
+  $ hg init fuzznotes
+  $ cd fuzznotes
+  $ touch fix1
+  $ hg -q commit -A -l - << EOF
+  > commit 1
+  > 
+  > .. fix::
+  > 
+  >    This is a fix with another line.
+  >    And it is a big one.
+  > EOF
+
+  $ cat >> $TESTTMP/fuzz-ignore-notes << EOF
+  > Bug Fixes
+  > =========
+  > 
+  > * Fixed issue4567 by improving X.
+  > 
+  > * This is the first line. This is next line with one newline.
+  > 
+  >   This is another line written after two newlines. This is going to be a big one.
+  > 
+  > * This fixes another problem.
+  > EOF
+
+  $ hg releasenotes -r . $TESTTMP/fuzz-ignore-notes
+  "This is a fix with another line. And it is a big one. " already exists in notes; ignoring
+
+  $ cat $TESTTMP/fuzz-ignore-notes
+  Bug Fixes
+  =========
+  
+  * Fixed issue4567 by improving X.
+  
+  * This is the first line. This is next line with one newline.
+  
+    This is another line written after two newlines. This is going to be a big
+    one.
+  
+  * This fixes another problem.
diff -r 6684514628b4 -r 1fc4f41fadcc tests/test-releasenotes-parsing.t
--- a/tests/test-releasenotes-parsing.t	Mon Jul 03 22:51:58 2017 +0200
+++ b/tests/test-releasenotes-parsing.t	Mon Jul 03 23:01:27 2017 +0200
@@ -1,3 +1,5 @@ 
+#require fuzzywuzzy
+
   $ cat >> $HGRCPATH << EOF
   > [extensions]
   > releasenotes=