[PATCH v4 00/11] Add new git-related helper to contrib

[PATCH v4 00/11] Add new git-related helper to contrib

[Felipe Contreras]

Hi,

Previously known as git-cc-cmd, I've renamed it git-related (tentatively). I
removed support for aliases, as I don't think it's very useful, and I've added
support for mailmap, which I think covers similar use-cases.

This script allows you to get a list of relevant persons to Cc when sending a
patch series.

  % git cc-cmd v1.8.1.6^^1..v1.8.1.6^^2
  Junio C Hamano <gitster@pobox.com> (signer: 84%, author: 15%)
  "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com> (author: 38%, signer: 7%)
  Michael Haggerty <mhagger@alum.mit.edu> (author: 15%)
  Jean-Noel Avila <jn.avila@free.fr> (signer: 7%)
  "Jean-Noël AVILA" <avila.jn@gmail.com> (author: 7%)
  "Henrik Grubbström" <grubba@grubba.org> (author: 7%)
  Clemens Buchacher <drizzd@aon.at> (author: 7%)
  Joshua Jensen <jjensen@workspacewhiz.com> (author: 7%)
  Johannes Sixt <j6t@kdbg.org> (signer: 7%)

It finds people that might be interesting in a patch, by going back through the
history for each single hunk modified, and finding people that reviewed,
acknowledge, signed, or authored the code the patch is modifying.

It does this by running 'git blame' incrementally on each hunk, and then
parsing the commit message. After gathering all the relevant people, it groups
them to show what exactly was their role when the participated in the
development of the relevant commit, and on how many relevant commits they
participated. They are only displayed if they pass a minimum threshold of
participation.

The code finds the changes in each commit in the list, runs 'git blame'
to see which other commits are relevant to those lines, and then adds
the author and signer to the list.

Finally, it calculates what percentage of the total relevant commits
each person was involved in, and if it passes the threshold, it goes in.

You can also choose to show the commits themselves:

  % git cc-cmd --commits v1.8.1.6^^1..v1.8.1.6^^2
  9db9eec attr: avoid calling find_basename() twice per path
  94bc671 Add directory pattern matching to attributes
  82dce99 attr: more matching optimizations from .gitignore
  593cb88 exclude: split basename matching code into a separate function
  b559263 exclude: split pathname matching code into a separate function
  4742d13 attr: avoid searching for basename on every match
  f950eb9 rename pathspec_prefix() to common_prefix() and move to dir.[ch]
  4a085b1 consolidate pathspec_prefix and common_prefix
  d932f4e Rename git_checkattr() to git_check_attr()
  2d72174 Extract a function collect_all_attrs()
  8cf2a84 Add string comparison functions that respect the ignore_case variable.
  407a963 Merge branch 'rr/remote-helper-doc'
  ec775c4 attr: Expand macros immediately when encountered.

But wait, there's more: you can also specify a list of patch files, which means
this can be used for 'git send-emails' --cc-cmd option.


Felipe Contreras (11):
  Add new git-related  helper to contrib
  contrib: related: add option parsing
  contrib: related: add support for multiple patches
  contrib: related: add option to show commits
  contrib: related: add option to parse from committish
  contrib: related: parse committish like format-patch
  contrib: related: fix parsing of rev-list args
  contrib: related: support multiple roles
  contrib: related: sort by participation
  contrib: related: group persons with same email
  contrib: related: add support for mailmap

 contrib/related/git-related | 272 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 272 insertions(+)
 create mode 100755 contrib/related/git-related

-- 
1.8.2.1

[PATCH v4 01/11] Add new git-related helper to contrib

[Felipe Contreras]

This script find people that might be interested in a patch, by going
back through the history for each single hunk modified, and finding
people that reviewed, acknowledge, signed, or authored the code the
patch is modifying.

It does this by running 'git blame' incrementally on each hunk, and then
parsing the commit message. After gathering all the relevant people, it
groups them to show what exactly was their role when the participated in
the development of the relevant commit, and on how many relevant commits
they participated. They are only displayed if they pass a minimum
threshold of participation.

For example:

  % git related 0001-remote-hg-trivial-cleanups.patch
  Felipe Contreras <felipe.contreras@gmail.com> (author: 100%)
  Jeff King <peff@peff.net> (signer: 83%)
  Max Horn <max@quendi.de> (signer: 16%)
  Junio C Hamano <gitster@pobox.com> (signer: 16%)

Thus it can be used for 'git send-email' as a cc-cmd.

There might be some other related functions to this script, not just to
be used as a cc-cmd.

Comments-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 131 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 131 insertions(+)
 create mode 100755 contrib/related/git-related

diff --git a/contrib/related/git-related b/contrib/related/git-related
new file mode 100755
index 0000000..2d47efa
--- /dev/null
+++ b/contrib/related/git-related
@@ -0,0 +1,131 @@
+#!/usr/bin/env ruby
+
+$since = '3-years-ago'
+$min_percent = 5
+
+class Commit
+
+  attr_reader :id, :roles
+
+  def initialize(id)
+    @id = id
+    @roles = []
+  end
+
+  def parse(data)
+    author = msg = nil
+    roles = {}
+    data.each_line do |line|
+      if not msg
+        case line
+        when /^author ([^<>]+) <(\S+)> (.+)$/
+          author = $1, $2
+          roles[author] = :author
+        when /^$/
+          msg = true
+        end
+      else
+        if line =~ /^(Signed-off|Reviewed|Acked)-by: ([^<>]+) <(\S+?)>$/
+          person = $2, $3
+          roles[person] = :signer if person != author
+        end
+      end
+    end
+    @roles = roles.map do |person, role|
+      [person, role]
+    end
+  end
+
+end
+
+class Commits
+
+  attr_reader :items
+
+  def initialize()
+    @items = {}
+  end
+
+  def size
+    @items.size
+  end
+
+  def import
+    return if @items.empty?
+    File.popen(%w[git cat-file --batch], 'r+') do |p|
+      p.write(@items.keys.join("\n"))
+      p.close_write
+      p.each do |l|
+        if l =~ /^(\h{40}) commit (\d+)/
+          id, len = $1, $2
+          data = p.read($2.to_i)
+          @items[id].parse(data)
+        end
+      end
+    end
+  end
+
+  def get_blame(source, start, offset, from)
+    return unless source and offset
+    File.popen(['git', 'blame', '--incremental', '-C',
+               '-L', '%u,+%u' % [start, offset],
+               '--since', $since, from + '^',
+               '--', source]) do |p|
+      p.each do |line|
+        if line =~ /^(\h{40})/
+          id = $1
+          @items[id] = Commit.new(id)
+        end
+      end
+    end
+  end
+
+  def from_patch(file)
+    source = nil
+    from = nil
+    File.open(file) do |f|
+      f.each do |line|
+        case line
+        when /^From (\h+) (.+)$/
+          from = $1
+        when /^---\s+(\S+)/
+          source = $1 != '/dev/null' ? $1[2..-1] : nil
+        when /^@@\s-(\d+),(\d+)/
+          get_blame(source, $1, $2, from)
+        end
+      end
+    end
+  end
+
+end
+
+exit 1 if ARGV.size != 1
+
+commits = Commits.new
+commits.from_patch(ARGV[0])
+commits.import
+
+# hash of hashes
+persons = Hash.new { |hash, key| hash[key] = {} }
+
+commits.items.values.each do |commit|
+  commit.roles.each do |person, role|
+    persons[person][role] ||= 0
+    persons[person][role] += 1
+  end
+end
+
+persons.each do |person, roles|
+  roles = roles.map do |role, count|
+    percent = count.to_f * 100 / commits.size
+    next if percent < $min_percent
+    '%s: %u%%' % [role, percent]
+  end.compact
+  next if roles.empty?
+
+  name, email = person
+  # must quote chars?
+  name = '"%s"' % name if name =~ /[^\w \-]/i
+  person = name ? '%s <%s>' % [name, email] : email
+  puts '%s (%s)' % [person, roles.join(', ')]
+end
-- 
1.8.2.1

[PATCH v4 02/11] contrib: related: add option parsing

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/contrib/related/git-related b/contrib/related/git-related
index 2d47efa..702836a 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -1,8 +1,25 @@
 #!/usr/bin/env ruby
 
+require 'optparse'
+
 $since = '3-years-ago'
 $min_percent = 5
 
+begin
+  OptionParser.new do |opts|
+    opts.program_name = 'git cc-cmd'
+    opts.banner = 'usage: git cc-cmd [options] <file>'
+
+    opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role participation') do |v|
+      $min_percent = v
+    end
+    opts.on('-d', '--since DATE', 'How far back to search for relevant commits') do |v|
+      $since = v
+    end
+  end.parse!
+rescue OptionParser::InvalidOption
+end
+
 class Commit
 
   attr_reader :id, :roles
-- 
1.8.2.1

[PATCH v4 03/11] contrib: related: add support for multiple patches

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/contrib/related/git-related b/contrib/related/git-related
index 702836a..90ec3aa 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -8,7 +8,7 @@ $min_percent = 5
 begin
   OptionParser.new do |opts|
     opts.program_name = 'git cc-cmd'
-    opts.banner = 'usage: git cc-cmd [options] <file>'
+    opts.banner = 'usage: git cc-cmd [options] <files>'
 
     opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role participation') do |v|
       $min_percent = v
@@ -61,6 +61,7 @@ class Commits
 
   def initialize()
     @items = {}
+    @main_commits = {}
   end
 
   def size
@@ -91,24 +92,27 @@ class Commits
       p.each do |line|
         if line =~ /^(\h{40})/
           id = $1
-          @items[id] = Commit.new(id)
+          @items[id] = Commit.new(id) if not @main_commits.include?(id)
         end
       end
     end
   end
 
-  def from_patch(file)
+  def from_patches(files)
     source = nil
-    from = nil
-    File.open(file) do |f|
-      f.each do |line|
-        case line
-        when /^From (\h+) (.+)$/
-          from = $1
-        when /^---\s+(\S+)/
-          source = $1 != '/dev/null' ? $1[2..-1] : nil
-        when /^@@\s-(\d+),(\d+)/
-          get_blame(source, $1, $2, from)
+    files.each do |file|
+      from = nil
+      File.open(file) do |f|
+        f.each do |line|
+          case line
+          when /^From (\h+) (.+)$/
+            from = $1
+            @main_commits[from] = true
+          when /^---\s+(\S+)/
+            source = $1 != '/dev/null' ? $1[2..-1] : nil
+          when /^@@\s-(\d+),(\d+)/
+            get_blame(source, $1, $2, from)
+          end
         end
       end
     end
@@ -116,10 +120,8 @@ class Commits
 
 end
 
-exit 1 if ARGV.size != 1
-
 commits = Commits.new
-commits.from_patch(ARGV[0])
+commits.from_patches(ARGV)
 commits.import
 
 # hash of hashes
-- 
1.8.2.1

[PATCH v4 04/11] contrib: related: add option to show commits

[Felipe Contreras]

Instead of showing the authors and signers, show the commits themselves.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/contrib/related/git-related b/contrib/related/git-related
index 90ec3aa..6eed4bc 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -4,6 +4,7 @@ require 'optparse'
 
 $since = '3-years-ago'
 $min_percent = 5
+$show_commits = false
 
 begin
   OptionParser.new do |opts|
@@ -16,6 +17,9 @@ begin
     opts.on('-d', '--since DATE', 'How far back to search for relevant commits') do |v|
       $since = v
     end
+    opts.on('-c', '--commits[=FORMAT]', [:raw, :full], 'List commits instead of persons') do |v|
+      $show_commits = v || true
+    end
   end.parse!
 rescue OptionParser::InvalidOption
 end
@@ -124,6 +128,20 @@ commits = Commits.new
 commits.from_patches(ARGV)
 commits.import
 
+if $show_commits
+  cmd = nil
+  case $show_commits
+  when :raw
+    puts commits.items.keys
+  when :full
+    cmd = %w[git log --patch --no-walk]
+  else
+    cmd = %w[git log --oneline --no-walk]
+  end
+  system(*cmd + commits.items.keys) if cmd
+  exit 0
+end
+
 # hash of hashes
 persons = Hash.new { |hash, key| hash[key] = {} }
 
-- 
1.8.2.1

[PATCH v4 05/11] contrib: related: add option to parse from committish

[Felipe Contreras]

For example master..feature-a.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/contrib/related/git-related b/contrib/related/git-related
index 6eed4bc..0015b3c 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -5,11 +5,13 @@ require 'optparse'
 $since = '3-years-ago'
 $min_percent = 5
 $show_commits = false
+$files = []
+$rev_args = []
 
 begin
   OptionParser.new do |opts|
     opts.program_name = 'git cc-cmd'
-    opts.banner = 'usage: git cc-cmd [options] <files>'
+    opts.banner = 'usage: git cc-cmd [options] <files | rev-list options>'
 
     opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role participation') do |v|
       $min_percent = v
@@ -122,10 +124,40 @@ class Commits
     end
   end
 
+  def from_rev_args(args)
+    return if args.empty?
+    source = nil
+    File.popen(%w[git rev-list --reverse] + args) do |p|
+      p.each do |e|
+        id = e.chomp
+        @main_commits[id] = true
+        File.popen(%w[git show -C --oneline] + [id]) do |p|
+          p.each do |e|
+            case e
+            when /^---\s+(\S+)/
+              source = $1 != '/dev/null' ? $1[2..-1] : nil
+            when /^@@\s-(\d+),(\d+)/
+              get_blame(source, $1, $2, id)
+            end
+          end
+        end
+      end
+    end
+  end
+
+end
+
+ARGV.each do |e|
+  if File.exists?(e)
+    $files << e
+  else
+    $rev_args << e
+  end
 end
 
 commits = Commits.new
-commits.from_patches(ARGV)
+commits.from_patches($files)
+commits.from_rev_args($rev_args)
 commits.import
 
 if $show_commits
-- 
1.8.2.1

[PATCH v4 06/11] contrib: related: parse committish like format-patch

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/contrib/related/git-related b/contrib/related/git-related
index 0015b3c..2f38ee1 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -126,6 +126,20 @@ class Commits
 
   def from_rev_args(args)
     return if args.empty?
+
+    revs = []
+
+    File.popen(%w[git rev-parse --revs-only --default=HEAD --symbolic] + args).each do |rev|
+      revs << rev.chomp
+    end
+
+    case revs.size
+    when 1
+      committish = [ '%s..HEAD' % revs[0] ]
+    else
+      committish = revs
+    end
+
     source = nil
     File.popen(%w[git rev-list --reverse] + args) do |p|
       p.each do |e|
-- 
1.8.2.1

[PATCH v4 07/11] contrib: related: fix parsing of rev-list args

[Felipe Contreras]

For example '-1'.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/contrib/related/git-related b/contrib/related/git-related
index 2f38ee1..e8603be 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -23,7 +23,8 @@ begin
       $show_commits = v || true
     end
   end.parse!
-rescue OptionParser::InvalidOption
+rescue OptionParser::InvalidOption => e
+  $rev_args += e.args
 end
 
 class Commit
@@ -135,9 +136,11 @@ class Commits
 
     case revs.size
     when 1
-      committish = [ '%s..HEAD' % revs[0] ]
+      r = revs[0]
+      r = '^' + r if r[0] != '-'
+      args = [ r, 'HEAD' ]
     else
-      committish = revs
+      args = revs
     end
 
     source = nil
-- 
1.8.2.1

[PATCH v4 08/11] contrib: related: support multiple roles

[Felipe Contreras]

Currently only the roles of 'author' and 'signer' and handler, but now
there's also 'reviewer' and 'acker'.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/contrib/related/git-related b/contrib/related/git-related
index e8603be..cf6818e 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -27,6 +27,12 @@ rescue OptionParser::InvalidOption => e
   $rev_args += e.args
 end
 
+KNOWN_ROLES = {
+  'Signed-off' => :signer,
+  'Reviewed' => :reviewer,
+  'Acked' => :acker,
+}
+
 class Commit
 
   attr_reader :id, :roles
@@ -38,25 +44,28 @@ class Commit
 
   def parse(data)
     author = msg = nil
-    roles = {}
+    # hash of arrays
+    roles = Hash.new { |hash, key| hash[key] = [] }
     data.each_line do |line|
       if not msg
         case line
         when /^author ([^<>]+) <(\S+)> (.+)$/
           author = $1, $2
-          roles[author] = :author
+          roles[author] << :author
         when /^$/
           msg = true
         end
       else
-        if line =~ /^(Signed-off|Reviewed|Acked)-by: ([^<>]+) <(\S+?)>$/
+        role_regex = KNOWN_ROLES.keys.join('|')
+        if line =~ /^(#{role_regex})-by: ([^<>]+) <(\S+?)>$/
           person = $2, $3
-          roles[person] = :signer if person != author
+          role = KNOWN_ROLES[$1]
+          roles[person] << role if person != author
         end
       end
     end
-    @roles = roles.map do |person, role|
-      [person, role]
+    @roles = roles.map do |person, roles|
+      [person, roles]
     end
   end
 
@@ -195,9 +204,11 @@ end
 persons = Hash.new { |hash, key| hash[key] = {} }
 
 commits.items.values.each do |commit|
-  commit.roles.each do |person, role|
-    persons[person][role] ||= 0
-    persons[person][role] += 1
+  commit.roles.each do |person, roles|
+    roles.each do |role|
+      persons[person][role] ||= 0
+      persons[person][role] += 1
+    end
   end
 end
 
-- 
1.8.2.1

[PATCH v4 09/11] contrib: related: sort by participation

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/contrib/related/git-related b/contrib/related/git-related
index cf6818e..4e9b916 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -212,7 +212,12 @@ commits.items.values.each do |commit|
   end
 end
 
-persons.each do |person, roles|
+# sort by number of participations
+count_sort = lambda do |a, b|
+  b[1].values.reduce(:+) <=> a[1].values.reduce(:+)
+end
+
+persons.sort(&count_sort).each do |person, roles|
   roles = roles.map do |role, count|
     percent = count.to_f * 100 / commits.size
     next if percent < $min_percent
-- 
1.8.2.1

[PATCH v4 10/11] contrib: related: group persons with same email

[Felipe Contreras]

We still need the name of the person, so it might make sense to have a
Person object to simplify the code. Later.

Suggested-by: Duy Nguyen <pclouds@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/contrib/related/git-related b/contrib/related/git-related
index 4e9b916..f85e924 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -202,12 +202,15 @@ end
 
 # hash of hashes
 persons = Hash.new { |hash, key| hash[key] = {} }
+names = {}
 
 commits.items.values.each do |commit|
   commit.roles.each do |person, roles|
+    name, email = person
+    names[email] ||= name
     roles.each do |role|
-      persons[person][role] ||= 0
-      persons[person][role] += 1
+      persons[email][role] ||= 0
+      persons[email][role] += 1
     end
   end
 end
@@ -217,7 +220,7 @@ count_sort = lambda do |a, b|
   b[1].values.reduce(:+) <=> a[1].values.reduce(:+)
 end
 
-persons.sort(&count_sort).each do |person, roles|
+persons.sort(&count_sort).each do |email, roles|
   roles = roles.map do |role, count|
     percent = count.to_f * 100 / commits.size
     next if percent < $min_percent
@@ -225,7 +228,7 @@ persons.sort(&count_sort).each do |person, roles|
   end.compact
   next if roles.empty?
 
-  name, email = person
+  name = names[email]
   # must quote chars?
   name = '"%s"' % name if name =~ /[^\w \-]/i
   person = name ? '%s <%s>' % [name, email] : email
-- 
1.8.2.1

[PATCH v4 11/11] contrib: related: add support for mailmap

[Felipe Contreras]

This seems to be the way git tools do it.

Suggested-by: Duy Nguyen <pclouds@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/contrib/related/git-related b/contrib/related/git-related
index f85e924..be263e2 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -7,6 +7,8 @@ $min_percent = 5
 $show_commits = false
 $files = []
 $rev_args = []
+$mailmaps = {}
+$mailmaps_complex = {}
 
 begin
   OptionParser.new do |opts|
@@ -27,6 +29,39 @@ rescue OptionParser::InvalidOption => e
   $rev_args += e.args
 end
 
+def get_mailmap(filename)
+  return unless File.exists?(filename)
+  File.open(filename) do |f|
+    f.each do |line|
+      case line.gsub(/\s*#.*$/, '')
+      when /^([^<>]+)\s+<(\S+)>$/
+        $mailmaps[$2] = [ $1, nil ]
+      when /^<(\S+)>\s+<(\S+)>$/
+        $mailmaps[$2] = [ nil, $1 ]
+      when /^([^<>]+)\s+<(\S+)>\s+<(\S+)>$/
+        $mailmaps[$3] = [ $1, $2 ]
+      when /^([^<>]+)\s+<(\S+)>\s+([^<>]+)\s+<(\S+)>$/
+        $mailmaps_complex[[$3, $4]] = [ $1, $2 ]
+      end
+    end
+  end
+end
+
+def mailmap_fix(person)
+  new = nil
+  name, email = person
+  new = $mailmaps_complex[person] if not new and $mailmaps_complex.include?(person)
+  new = $mailmaps[email] if not new and $mailmaps.include?(email)
+  return if not new
+  person[0] = new[0] if new[0]
+  person[1] = new[1] if new[1]
+end
+
+get_aliases if $get_aliases
+get_mailmap('.mailmap')
+mailmap_file = %x[git config mailmap.file].chomp
+get_mailmap(mailmap_file)
+
 KNOWN_ROLES = {
   'Signed-off' => :signer,
   'Reviewed' => :reviewer,
@@ -65,6 +100,7 @@ class Commit
       end
     end
     @roles = roles.map do |person, roles|
+      mailmap_fix(person)
       [person, roles]
     end
   end
-- 
1.8.2.1

Re: [PATCH v4 01/11] Add new git-related helper to contrib

[Felipe Contreras]

Hi,

On Thu, Apr 25, 2013 at 2:59 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> This script find people that might be interested in a patch, by going
> back through the history for each single hunk modified, and finding
> people that reviewed, acknowledge, signed, or authored the code the
> patch is modifying.
>
> It does this by running 'git blame' incrementally on each hunk, and then
> parsing the commit message. After gathering all the relevant people, it
> groups them to show what exactly was their role when the participated in
> the development of the relevant commit, and on how many relevant commits
> they participated. They are only displayed if they pass a minimum
> threshold of participation.

Is this patch still not understandable? If so, I would gladly strip
away functionality, like the ability to show the roles. But for the
functionality it provides, I don't see how it could be any simpler.

Sure, code comments might help, but first I would like to make the
code as self-documenting as possible, so I would give a try at a
simplified version first, and then perhaps adding a Person object.

Better for me if the code was good enough as it is though.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v4 01/11] Add new git-related helper to contrib

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Thu, Apr 25, 2013 at 2:59 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> This script find people that might be interested in a patch, by going
>> back through the history for each single hunk modified, and finding
>> people that reviewed, acknowledge, signed, or authored the code the
>> patch is modifying.
>>
>> It does this by running 'git blame' incrementally on each hunk, and then
>> parsing the commit message. After gathering all the relevant people, it
>> groups them to show what exactly was their role when the participated in
>> the development of the relevant commit, and on how many relevant commits
>> they participated. They are only displayed if they pass a minimum
>> threshold of participation.
>
> Is this patch still not understandable?

Among the people who review patches here and give usable feedback,
earlier this week Peff said he is away from his mailbox for the rest
of the week, and I am not reviewing any new topics that are not in
'next', being busy in preparation for -rc0, so I wouldn't be able to
answer that question.

I do not know about the others, but it is understandable from time
to time there is a period a series is not being reviewed by anybody.

Re: [PATCH v4 01/11] Add new git-related helper to contrib

[Felipe Contreras]

On Fri, Apr 26, 2013 at 6:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, Apr 25, 2013 at 2:59 PM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> This script find people that might be interested in a patch, by going
>>> back through the history for each single hunk modified, and finding
>>> people that reviewed, acknowledge, signed, or authored the code the
>>> patch is modifying.
>>>
>>> It does this by running 'git blame' incrementally on each hunk, and then
>>> parsing the commit message. After gathering all the relevant people, it
>>> groups them to show what exactly was their role when the participated in
>>> the development of the relevant commit, and on how many relevant commits
>>> they participated. They are only displayed if they pass a minimum
>>> threshold of participation.
>>
>> Is this patch still not understandable?
>
> Among the people who review patches here and give usable feedback,
> earlier this week Peff said he is away from his mailbox for the rest
> of the week, and I am not reviewing any new topics that are not in
> 'next', being busy in preparation for -rc0, so I wouldn't be able to
> answer that question.
>
> I do not know about the others, but it is understandable from time
> to time there is a period a series is not being reviewed by anybody.

That's fine, I was mostly asking Ramkumar who earlier argued earlier
versions of this patch were not understandable.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v4 01/11] Add new git-related helper to contrib

[Ramkumar Ramachandra]

Felipe Contreras wrote:
> That's fine, I was mostly asking Ramkumar who earlier argued earlier
> versions of this patch were not understandable.

Sorry, still catching up with list emails.  At a glance, part 1 looks
much better.  Will read through more carefully soon.

Thanks.

Re: [PATCH v4 01/11] Add new git-related helper to contrib

[Felipe Contreras]

On Fri, Apr 26, 2013 at 10:01 PM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Felipe Contreras wrote:
>> That's fine, I was mostly asking Ramkumar who earlier argued earlier
>> versions of this patch were not understandable.
>
> Sorry, still catching up with list emails.  At a glance, part 1 looks
> much better.  Will read through more carefully soon.

Gentle reminder.

-- 
Felipe Contreras