You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@whimsical.apache.org by se...@apache.org on 2022/02/09 13:23:03 UTC

[whimsy] branch master updated: General tidy up

This is an automated email from the ASF dual-hosted git repository.

sebb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/whimsy.git


The following commit(s) were added to refs/heads/master by this push:
     new 0584b42  General tidy up
0584b42 is described below

commit 0584b421580e0f1c8c9a2181329e32f0e76a9668
Author: Sebb <se...@apache.org>
AuthorDate: Wed Feb 9 13:22:57 2022 +0000

    General tidy up
---
 www/roster/models/committee.rb | 39 ++++++++++++++++--------------------
 www/roster/models/nonpmc.rb    | 45 +++++++++++++++++++-----------------------
 www/roster/models/ppmc.rb      | 30 +++++++++++++---------------
 3 files changed, 51 insertions(+), 63 deletions(-)

diff --git a/www/roster/models/committee.rb b/www/roster/models/committee.rb
index b4c0df9..d2951e2 100644
--- a/www/roster/models/committee.rb
+++ b/www/roster/models/committee.rb
@@ -1,6 +1,5 @@
 class Committee
   def self.serialize(id, env)
-    response = {}
 
     pmc = ASF::Committee.find(id)
     return unless pmc.pmc? # Only show PMCs
@@ -10,9 +9,9 @@ class Committee
 
     ASF::Committee.load_committee_info
     # We'll be needing the mail data later
-    people = ASF::Person.preload(['cn', 'mail', 'asf-altEmail', 'githubUsername'], (members + committers).uniq)
+    ASF::Person.preload(['cn', 'mail', 'asf-altEmail', 'githubUsername'], (members + committers).uniq)
 
-    lists = ASF::Mail.lists(true).select do |list, mode|
+    lists = ASF::Mail.lists(true).select do |list, _|
       list =~ /^#{pmc.mail_list}\b/
     end
 
@@ -25,7 +24,7 @@ class Committee
     modtime = nil
     subscribers = nil # we get the counts only here
     subtime = nil
-    pSubs = Array.new # private@ subscribers
+    pSubs = [] # private@ subscribers
     unMatchedSubs = [] # unknown private@ subscribers
     unMatchedSecSubs = [] # unknown security@ subscribers
     currentUser = ASF::Person.find(env.user)
@@ -44,17 +43,17 @@ class Committee
       if analysePrivateSubs
         pSubs = ASF::MLIST.private_subscribers(pmc.mail_list)[0]||[]
         unMatchedSubs=Set.new(pSubs) # init ready to remove matched mails
-        pSubs.map!{|m| m.downcase} # for matching
+        pSubs.map!(&:downcase) # for matching
         sSubs = ASF::MLIST.security_subscribers(pmc.mail_list)[0]||[]
         unMatchedSecSubs=Set.new(sSubs) # init ready to remove matched mails
       end
     else
-      lists = lists.select {|list, mode| mode == 'public'}
+      lists = lists.select {|_, mode| mode == 'public'}
     end
 
     roster = pmc.roster.dup # from committee-info
     # ensure PMC members are all processed even they don't belong to the owner group
-    roster.each do |key, info|
+    roster.each do |key, _|
       info[:role] = 'PMC member'
       next if pmc.ownerids.include?(key) # skip the rest (expensive) if person is in the owner group
       person = ASF::Person[key]
@@ -97,7 +96,7 @@ class Committee
       roster[person.id]['githubUsername'] = (person.attrs['githubUsername'] || []).join(', ')
     end
 
-    roster.each {|id, info| info[:member] = ASF::Person.find(id).asf_member?}
+    roster.each {|k, v| v[:member] = ASF::Person.find(k).asf_member?}
 
     if pmc.chair and roster[pmc.chair.id]
       roster[pmc.chair.id]['role'] = 'PMC chair'
@@ -108,13 +107,11 @@ class Committee
     asfMembers = []
     unknownSecSubs = [] # unknown security@ subscribers: not PMC or ASF
     # Also look for non-ASF mod emails
-    nonASFmails=Hash.new
-    if moderators
-      moderators.each { |list,mods| mods.each {|m| nonASFmails[m]='' unless m.end_with? '@apache.org'} }
-    end
+    nonASFmails = {}
+    moderators?.each { |_, mods| mods.each {|m| nonASFmails[m] = '' unless m.end_with? '@apache.org'} }
     if unMatchedSubs.length > 0 or nonASFmails.length > 0 or unMatchedSecSubs.length > 0
       load_emails # set up @people
-      unMatchedSubs.each{ |addr|
+      unMatchedSubs.each { |addr|
         who = nil
         @people.each do |person|
           if person[:mail].any? {|mail| mail.downcase == addr.downcase}
@@ -131,14 +128,14 @@ class Committee
           unknownSubs << { addr: addr, person: nil }
         end
       }
-      nonASFmails.each {|k,v|
+      nonASFmails.each {|k, _|
         @people.each do |person|
           if person[:mail].any? {|mail| ASF::Mail.to_canonical(mail.downcase) == ASF::Mail.to_canonical(k.downcase)}
             nonASFmails[k] = person[:id]
           end
         end
       }
-      unMatchedSecSubs.each{ |addr|
+      unMatchedSecSubs.each { |addr|
         who = nil
         @people.each do |person|
           if person[:mail].any? {|mail| mail.downcase == addr.downcase}
@@ -160,9 +157,9 @@ class Committee
       pmcchairs = ASF::Service.find('pmc-chairs')
       pmc_chair = pmcchairs.members.include? pmc.chair
     end
-    response = {
+    return {
       id: id,
-      chair: pmc.chair && pmc.chair.id,
+      chair: pmc.chair?.id,
       pmc_chair: pmc_chair,
       display_name: pmc.display_name,
       description: pmc.description,
@@ -174,7 +171,7 @@ class Committee
       members: pmc.roster.keys,
       committers: committers.map(&:id),
       roster: roster,
-      mail: Hash[lists.sort],
+      mail: lists.sort.to_h,
       moderators: moderators,
       modtime: modtime,
       subscribers: subscribers,
@@ -188,7 +185,6 @@ class Committee
       unknownSecSubs: unknownSecSubs,
     }
 
-    response
   end
 
   private
@@ -197,10 +193,9 @@ class Committee
     # recompute index if the data is 5 minutes old or older
     @people = nil if not @people_time or Time.now-@people_time >= 300
 
-    if not @people
+    unless @people
       # bulk loading the mail information makes things go faster
-      mail = Hash[ASF::Mail.list.group_by(&:last).
-        map {|person, list| [person, list.map(&:first)]}]
+      mail = ASF::Mail.list.group_by(&:last).transform_values {|list| list.map(&:first)}
 
       # build a list of people, their public-names, and email addresses
       @people = ASF::Person.list.map {|person|
diff --git a/www/roster/models/nonpmc.rb b/www/roster/models/nonpmc.rb
index cb53aef..144511c 100644
--- a/www/roster/models/nonpmc.rb
+++ b/www/roster/models/nonpmc.rb
@@ -1,6 +1,5 @@
 class NonPMC
   def self.serialize(id, env)
-    response = {}
 
     cttee = ASF::Committee.find(id)
     return unless cttee.nonpmc?
@@ -9,14 +8,14 @@ class NonPMC
     committers = cttee.committers
     # Hack to fix unusual mail_list values e.g. press@apache.org
     mail_list = cttee.mail_list.sub(/@.*/,'')
-    mail_list = 'legal' if mail_list =~ /^legal-/ unless cttee.name == 'dataprivacy'
+    mail_list = 'legal' if mail_list =~ /^legal-/ && cttee.name != 'dataprivacy'
     mail_list = 'fundraising' if mail_list =~ /^fundraising-/
 
     ASF::Committee.load_committee_info
     # We'll be needing the mail data later
-    people = ASF::Person.preload(['cn', 'mail', 'asf-altEmail', 'githubUsername'], (members + committers).uniq)
+    ASF::Person.preload(['cn', 'mail', 'asf-altEmail', 'githubUsername'], (members + committers).uniq)
 
-    lists = ASF::Mail.lists(true).select do |list, mode|
+    lists = ASF::Mail.lists(true).select do |list, _|
       list =~ /^#{mail_list}\b/
     end
 
@@ -26,7 +25,7 @@ class NonPMC
     modtime = nil
     subscribers = nil # we get the counts only here
     subtime = nil
-    pSubs = Array.new # private@ subscribers
+    pSubs = [] # private@ subscribers
     unMatchedSubs = [] # unknown private@ subscribers
     unMatchedSecSubs = [] # unknown security@ subscribers
     currentUser = ASF::Person.find(env.user)
@@ -45,12 +44,12 @@ class NonPMC
       if analysePrivateSubs
         pSubs = ASF::MLIST.private_subscribers(mail_list)[0]||[]
         unMatchedSubs=Set.new(pSubs) # init ready to remove matched mails
-        pSubs.map!{|m| m.downcase} # for matching
+        pSubs.map!(&:downcase) # for matching
         sSubs = ASF::MLIST.security_subscribers(mail_list)[0]||[]
-        unMatchedSecSubs=Set.new(sSubs) # init ready to remove matched mails
+        unMatchedSecSubs = Set.new(sSubs) # init ready to remove matched mails
       end
     else
-      lists = lists.select {|list, mode| mode == 'public'}
+      lists = lists.select {|_, mode| mode == 'public'}
     end
 
     roster = cttee.roster.dup
@@ -63,7 +62,7 @@ class NonPMC
     cttee_members = roster.keys # get the potentially updated list
 
     # now add the status info
-    roster.each {|key, info| info[:role] = 'Committee member'}
+    roster.each {|_, info| info[:role] = 'Committee member'}
 
     members.each do |person|
       roster[person.id] ||= {
@@ -71,7 +70,7 @@ class NonPMC
         role: 'Committee member'
       }
       if analysePrivateSubs
-        allMail = person.all_mail.map{|m| m.downcase}
+        allMail = person.all_mail.map(&:downcase)
         roster[person.id]['notSubbed'] = (allMail & pSubs).empty?
         unMatchedSubs.delete_if {|k| allMail.include? k.downcase}
         unMatchedSecSubs.delete_if {|k| allMail.include? k.downcase}
@@ -88,7 +87,7 @@ class NonPMC
       roster[person.id]['githubUsername'] = (person.attrs['githubUsername'] || []).join(', ')
     end
 
-    roster.each {|id, info| info[:member] = ASF::Person.find(id).asf_member?}
+    roster.each {|_, info| info[:member] = ASF::Person.find(id).asf_member?}
 
     if cttee.chair and roster[cttee.chair.id]
       roster[cttee.chair.id]['role'] = 'Committee chair'
@@ -99,13 +98,11 @@ class NonPMC
     asfMembers = []
     unknownSecSubs = [] # unknown security@ subscribers: not PMC or ASF
     # Also look for non-ASF mod emails
-    nonASFmails=Hash.new
-    if moderators
-      moderators.each { |list,mods| mods.each {|m| nonASFmails[m]='' unless m.end_with? '@apache.org'} }
-    end
+    nonASFmails = {}
+    moderators?.each { |_, mods| mods.each {|m| nonASFmails[m] = '' unless m.end_with? '@apache.org'} }
     if unMatchedSubs.length > 0 or nonASFmails.length > 0 or unMatchedSecSubs.length > 0
       load_emails # set up @people
-      unMatchedSubs.each{ |addr|
+      unMatchedSubs.each { |addr|
         who = nil
         @people.each do |person|
           if person[:mail].any? {|mail| mail.downcase == addr.downcase}
@@ -122,14 +119,14 @@ class NonPMC
           unknownSubs << { addr: addr, person: nil }
         end
       }
-      nonASFmails.each {|k,v|
+      nonASFmails.each {|k, v|
         @people.each do |person|
           if person[:mail].any? {|mail| ASF::Mail.to_canonical(mail.downcase) == ASF::Mail.to_canonical(k.downcase)}
             nonASFmails[k] = person[:id]
           end
         end
       }
-      unMatchedSecSubs.each{ |addr|
+      unMatchedSecSubs.each { |addr|
         who = nil
         @people.each do |person|
           if person[:mail].any? {|mail| mail.downcase == addr.downcase}
@@ -146,9 +143,9 @@ class NonPMC
       }
     end
 
-    response = {
+    return {
       id: id,
-      chair: cttee.chair && cttee.chair.id,
+      chair: cttee.chair?.id,
       display_name: cttee.display_name,
       description: cttee.description,
       schedule: cttee.schedule,
@@ -160,7 +157,7 @@ class NonPMC
       members: cttee_members,
       committers: committers.map(&:id),
       roster: roster,
-      mail: Hash[lists.sort],
+      mail: lists.sort.to_h,
       moderators: moderators,
       modtime: modtime,
       subscribers: subscribers,
@@ -173,7 +170,6 @@ class NonPMC
       unknownSecSubs: unknownSecSubs,
     }
 
-    response
   end
 
   private
@@ -182,10 +178,9 @@ class NonPMC
     # recompute index if the data is 5 minutes old or older
     @people = nil if not @people_time or Time.now-@people_time >= 300
 
-    if not @people
+    unless @people
       # bulk loading the mail information makes things go faster
-      mail = Hash[ASF::Mail.list.group_by(&:last).
-        map {|person, list| [person, list.map(&:first)]}]
+      mail = ASF::Mail.list.group_by(&:last).transform_values {|list| list.map(&:first)}
 
       # build a list of people, their public-names, and email addresses
       @people = ASF::Person.list.map {|person|
diff --git a/www/roster/models/ppmc.rb b/www/roster/models/ppmc.rb
index 8c43ae6..69b1059 100644
--- a/www/roster/models/ppmc.rb
+++ b/www/roster/models/ppmc.rb
@@ -1,11 +1,10 @@
 class PPMC
   def self.serialize(id, env)
-    response = {}
 
     ppmc = ASF::Podling.find(id)
     return unless ppmc # Not found
 
-    lists = ASF::Mail.lists(true).select do |list, mode|
+    lists = ASF::Mail.lists(true).select do |list, _|
       list =~ /^(incubator-)?#{ppmc.mail_list}\b/
     end
 
@@ -16,13 +15,13 @@ class PPMC
     unknownSubs = []
     asfMembers = []
     # Also look for non-ASF mod emails
-    nonASFmails=Hash.new
+    nonASFmails = {}
 
     moderators = nil
     modtime = nil
     subscribers = nil # we get the counts only here
     subtime = nil
-    pSubs = Array.new # private@ subscribers
+    pSubs = [] # private@ subscribers
     unMatchedSubs = [] # unknown private@ subscribers
     currentUser = ASF::Person.find(env.user)
     analysePrivateSubs = false # whether to show missing private@ subscriptions
@@ -40,12 +39,12 @@ class PPMC
       if analysePrivateSubs
         pSubs = ASF::MLIST.private_subscribers(ppmc.mail_list)[0]||[]
         unMatchedSubs=Set.new(pSubs) # init ready to remove matched mails
-        pSubs.map!{|m| m.downcase} # for matching
+        pSubs.map!(&:downcase) # for matching
       end
 
-      moderators.each { |list,mods| mods.each {|m| nonASFmails[m]='' unless m.end_with? '@apache.org'} }
+      moderators.each { |_, mods| mods.each {|m| nonASFmails[m]='' unless m.end_with? '@apache.org'} }
     else
-      lists = lists.select {|list, mode| mode == 'public'}
+      lists = lists.select {|_, mode| mode == 'public'}
     end
 
     pmc = ASF::Committee.find('incubator')
@@ -130,7 +129,7 @@ class PPMC
       }
     end
 
-    response = {
+    return {
       id: id,
       display_name: ppmc.display_name,
       description: ppmc.description,
@@ -142,10 +141,10 @@ class PPMC
       status: ppmc.status,
       mentors: ppmc.mentors,
       hasLDAP: ppmc.hasLDAP?,
-      owners: owners.map {|person| person.id},
-      committers: committers.map {|person| person.id},
+      owners: owners.map(&:id),
+      committers: committers.map(&:id),
       roster: roster,
-      mail: Hash[lists.sort],
+      mail: lists.sort.to_h,
       moderators: moderators,
       modtime: modtime,
       subscribers: subscribers,
@@ -159,19 +158,18 @@ class PPMC
       asfMembers: asfMembers,
     }
 
-    response
   end
 
   private
 
   def self.load_emails
     # recompute index if the data is 5 minutes old or older
-    @people = nil if not @people_time or Time.now-@people_time >= 300
+    @people = nil if not @people_time or Time.now - @people_time >= 300
 
-    if not @people
+    unless @people
       # bulk loading the mail information makes things go faster
-      mail = Hash[ASF::Mail.list.group_by(&:last).
-        map {|person, list| [person, list.map(&:first)]}]
+      # TODO: it is still expensive
+      mail = ASF::Mail.list.group_by(&:last).transform_values {|list| list.map(&:first)}
 
       # build a list of people, their public-names, and email addresses
       @people = ASF::Person.list.map {|person|