reorganaize issues fetching in more clear way

lib/github_changelog_generator/generator/generator.rb

@@ -20,18 +20,35 @@ module GitHubChangelogGenerator
       @options = options
 
       @fetcher = GitHubChangelogGenerator::Fetcher.new @options
+
+      fetch_tags
+
+      fetch_issues_and_pr
+    end
+
+    def fetch_issues_and_pr
+      issues, pull_requests = @fetcher.fetch_closed_issues_and_pr
+
+      @pull_requests = @options[:pulls] ? get_filtered_pull_requests(pull_requests) : []
+
+      @issues = @options[:issues] ? get_filtered_issues(issues) : []
+
+      fetch_events_for_issues_and_pr
+      detect_actual_closed_dates(@issues + @pull_requests)
+    end
+
+    def fetch_tags
       # @all_tags = get_filtered_tags
       @all_tags = @fetcher.get_all_tags
 
-      # TODO: refactor this double asssign of @issues and @pull_requests and move all logic in one method
-      @issues, @pull_requests = @fetcher.fetch_closed_issues_and_pr
+      fetch_tags_dates
+      sort_tags_by_date
+    end
 
-      @pull_requests = @options[:pulls] ? get_filtered_pull_requests : []
-
-      @issues = @options[:issues] ? get_filtered_issues : []
-
-      fetch_event_for_issues_and_pr
-      detect_actual_closed_dates
+    # Sort all tags by date
+    def sort_tags_by_date
+      puts "Sorting tags..." if @options[:verbose]
+      @all_tags.sort_by! { |x| @fetcher.get_time_of_tag(x) }.reverse!
     end
 
     # Encapsulate characters to make markdown look as expected.

lib/github_changelog_generator/generator/generator_fetcher.rb

@@ -2,7 +2,7 @@ module GitHubChangelogGenerator
   class Generator
     # Fetch event for issues and pull requests
     # @return [Array] array of fetched issues
-    def fetch_event_for_issues_and_pr
+    def fetch_events_for_issues_and_pr
       if @options[:verbose]
         print "Fetching events for issues and PR: 0/#{@issues.count + @pull_requests.count}\r"
       end
@@ -37,28 +37,24 @@ module GitHubChangelogGenerator
     end
 
     # Find correct closed dates, if issues was closed by commits
-    def detect_actual_closed_dates
+    def detect_actual_closed_dates(issues)
       print "Fetching closed dates for issues...\r" if @options[:verbose]
 
+      # TODO: implement async fetching with slice!
       threads = []
 
-      @issues.each do |issue|
+      issues.each do |issue|
         threads << Thread.new do
           find_closed_date_by_commit(issue)
         end
       end
 
-      @pull_requests.each do |pull_request|
-        threads << Thread.new do
-          find_closed_date_by_commit(pull_request)
-        end
-      end
       threads.each(&:join)
 
       puts "Fetching closed dates for issues: Done!" if @options[:verbose]
     end
 
-    # Fill :actual_date parameter of specified issue by closed date of the commit, it it was closed by commit.
+    # Fill :actual_date parameter of specified issue by closed date of the commit, if it was closed by commit.
     # @param [Hash] issue
     def find_closed_date_by_commit(issue)
       unless issue["events"].nil?
@@ -67,22 +63,30 @@ module GitHubChangelogGenerator
         # reverse! - to find latest closed event. (event goes in date order)
         issue["events"].reverse!.each do |event|
           if event[:event].eql? compare_string
-            if event[:commit_id].nil?
-              issue[:actual_date] = issue[:closed_at]
-            else
-              begin
-                commit = @fetcher.fetch_commit(event)
-                issue[:actual_date] = commit[:author][:date]
-              rescue
-                puts "Warning: Can't fetch commit #{event[:commit_id]}. It is probably referenced from another repo.".yellow
-                issue[:actual_date] = issue[:closed_at]
-              end
-            end
+            set_date_from_event(event, issue)
             break
           end
         end
       end
       # TODO: assert issues, that remain without 'actual_date' hash for some reason.
     end
+
+    # Set closed date from this issue
+    #
+    # @param [Hash] event
+    # @param [Hash] issue
+    def set_date_from_event(event, issue)
+      if event[:commit_id].nil?
+        issue[:actual_date] = issue[:closed_at]
+      else
+        begin
+          commit = @fetcher.fetch_commit(event)
+          issue[:actual_date] = commit[:author][:date]
+        rescue
+          puts "Warning: Can't fetch commit #{event[:commit_id]}. It is probably referenced from another repo.".yellow
+          issue[:actual_date] = issue[:closed_at]
+        end
+      end
+    end
   end
 end

lib/github_changelog_generator/generator/generator_generation.rb

@@ -109,20 +109,9 @@ module GitHubChangelogGenerator
     # The full cycle of generation for whole project
     # @return [String] The complete change log
     def generate_log_for_all_tags
-      fetch_tags_dates
-
-      puts "Sorting tags..." if @options[:verbose]
-
-      @all_tags.sort_by! { |x| @fetcher.get_time_of_tag(x) }.reverse!
-
       puts "Generating log..." if @options[:verbose]
 
-      log = ""
-
-      if @options[:unreleased] && @all_tags.count != 0
-        unreleased_log = generate_log_between_tags(all_tags[0], nil)
-        log += unreleased_log if unreleased_log
-      end
+      log = generate_unreleased_section
 
       (1...all_tags.size).each do |index|
         log += generate_log_between_tags(all_tags[index], all_tags[index - 1])
@@ -134,6 +123,15 @@ module GitHubChangelogGenerator
       log
     end
 
+    def generate_unreleased_section
+      log = ""
+      if @options[:unreleased] && @all_tags.count != 0
+        unreleased_log = generate_log_between_tags(all_tags[0], nil)
+        log += unreleased_log if unreleased_log
+      end
+      log
+    end
+
     # Parse issue and generate single line formatted issue line.
     #
     # Example output:

lib/github_changelog_generator/generator/generator_processor.rb

@@ -85,45 +85,6 @@ module GitHubChangelogGenerator
       end
     end
 
-    # This method fetches missing params for PR and filter them by specified options
-    # It include add all PR's with labels from @options[:include_labels] array
-    # And exclude all from :exclude_labels array.
-    # @return [Array] filtered PR's
-    def get_filtered_pull_requests
-      filter_merged_pull_requests
-
-      filtered_pull_requests = include_issues_by_labels(@pull_requests)
-
-      filtered_pull_requests = exclude_issues_by_labels(filtered_pull_requests)
-
-      if @options[:verbose]
-        puts "Filtered pull requests: #{filtered_pull_requests.count}"
-      end
-
-      filtered_pull_requests
-    end
-
-    # This method filter only merged PR and
-    # fetch missing required attributes for pull requests
-    # :merged_at - is a date, when issue PR was merged.
-    # More correct to use merged date, rather than closed date.
-    def filter_merged_pull_requests
-      print "Fetching merged dates...\r" if @options[:verbose]
-      pull_requests = @fetcher.fetch_closed_pull_requests
-
-      @pull_requests.each do |pr|
-        fetched_pr = pull_requests.find do |fpr|
-          fpr.number == pr.number
-        end
-        pr[:merged_at] = fetched_pr[:merged_at]
-        pull_requests.delete(fetched_pr)
-      end
-
-      @pull_requests.select! do |pr|
-        !pr[:merged_at].nil?
-      end
-    end
-
     # Include issues with labels, specified in :include_labels
     # @param [Array] issues to filter
     # @return [Array] filtered array of issues
@@ -159,16 +120,55 @@ module GitHubChangelogGenerator
       filtered_tags
     end
 
+    # General filtered function
+    #
+    # @param [Array] all_issues
+    # @return [Array] filtered issues
+    def filter_array_by_labels(all_issues)
+      filtered_issues = include_issues_by_labels(all_issues)
+      exclude_issues_by_labels(filtered_issues)
+    end
+
     # Filter issues according labels
     # @return [Array] Filtered issues
-    def get_filtered_issues
-      filtered_issues = include_issues_by_labels(@issues)
+    def get_filtered_issues(issues)
+      issues = filter_array_by_labels(issues)
+      puts "Filtered issues: #{issues.count}" if @options[:verbose]
+      issues
+    end
 
-      filtered_issues = exclude_issues_by_labels(filtered_issues)
+    # This method fetches missing params for PR and filter them by specified options
+    # It include add all PR's with labels from @options[:include_labels] array
+    # And exclude all from :exclude_labels array.
+    # @return [Array] filtered PR's
+    def get_filtered_pull_requests(pull_requests)
+      pull_requests = filter_array_by_labels(pull_requests)
+      pull_requests = filter_merged_pull_requests(pull_requests)
+      puts "Filtered pull requests: #{pull_requests.count}" if @options[:verbose]
+      pull_requests
+    end
 
-      puts "Filtered issues: #{filtered_issues.count}" if @options[:verbose]
+    # This method filter only merged PR and
+    # fetch missing required attributes for pull requests
+    # :merged_at - is a date, when issue PR was merged.
+    # More correct to use merged date, rather than closed date.
+    def filter_merged_pull_requests(pull_requests)
+      print "Fetching merged dates...\r" if @options[:verbose]
+      closed_pull_requests = @fetcher.fetch_closed_pull_requests
 
-      filtered_issues
+      pull_requests.each do |pr|
+        fetched_pr = closed_pull_requests.find do |fpr|
+          fpr.number == pr.number
+        end
+        pr[:merged_at] = fetched_pr[:merged_at]
+        closed_pull_requests.delete(fetched_pr)
+      end
+
+      pull_requests.select! do |pr|
+        !pr[:merged_at].nil?
+      end
+
+      pull_requests
     end
   end
 end