From b10707b259392a87493e94127c7ddf6a59002fca Mon Sep 17 00:00:00 2001 From: Petr Korolev Date: Fri, 22 May 2015 17:44:06 +0300 Subject: [PATCH] reorganaize issues fetching in more clear way --- .rubocop_todo.yml | 18 ++-- lib/github_changelog_generator.rb | 1 + .../generator/generator.rb | 33 +++++-- .../generator/generator_fetcher.rb | 44 +++++----- .../generator/generator_generation.rb | 22 +++-- .../generator/generator_processor.rb | 88 +++++++++---------- 6 files changed, 113 insertions(+), 93 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 96a57ef..784052c 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,28 +1,28 @@ # This configuration was generated by `rubocop --auto-gen-config` -# on 2015-05-22 15:59:03 +0300 using RuboCop version 0.31.0. +# on 2015-05-22 17:34:14 +0300 using RuboCop version 0.31.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 17 +# Offense count: 16 Metrics/AbcSize: Max: 73 -# Offense count: 2 +# Offense count: 1 Metrics/BlockNesting: Max: 4 -# Offense count: 3 +# Offense count: 4 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 280 + Max: 162 -# Offense count: 5 +# Offense count: 4 Metrics/CyclomaticComplexity: Max: 15 -# Offense count: 23 +# Offense count: 21 # Configuration parameters: CountComments. Metrics/MethodLength: Max: 121 @@ -31,7 +31,7 @@ Metrics/MethodLength: Metrics/PerceivedComplexity: Max: 18 -# Offense count: 4 +# Offense count: 2 Style/AccessorMethodName: Enabled: false @@ -44,7 +44,7 @@ Style/Documentation: Style/GuardClause: Enabled: false -# Offense count: 2 +# Offense count: 1 # Configuration parameters: EnforcedStyle, MinBodyLength, SupportedStyles. Style/Next: Enabled: false diff --git a/lib/github_changelog_generator.rb b/lib/github_changelog_generator.rb index dea6253..c35db79 100755 --- a/lib/github_changelog_generator.rb +++ b/lib/github_changelog_generator.rb @@ -10,6 +10,7 @@ require_relative "github_changelog_generator/generator/generator" require_relative "github_changelog_generator/version" require_relative "github_changelog_generator/reader" +# The main module, where placed all classes (now, at least) module GitHubChangelogGenerator # Main class and entry point for this script. class ChangelogGenerator diff --git a/lib/github_changelog_generator/generator/generator.rb b/lib/github_changelog_generator/generator/generator.rb index 1799958..5b4285f 100644 --- a/lib/github_changelog_generator/generator/generator.rb +++ b/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. diff --git a/lib/github_changelog_generator/generator/generator_fetcher.rb b/lib/github_changelog_generator/generator/generator_fetcher.rb index bf7a2ef..5e65664 100644 --- a/lib/github_changelog_generator/generator/generator_fetcher.rb +++ b/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 diff --git a/lib/github_changelog_generator/generator/generator_generation.rb b/lib/github_changelog_generator/generator/generator_generation.rb index c5227d9..66f2a05 100644 --- a/lib/github_changelog_generator/generator/generator_generation.rb +++ b/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: diff --git a/lib/github_changelog_generator/generator/generator_processor.rb b/lib/github_changelog_generator/generator/generator_processor.rb index 55e736c..cbbaa7f 100644 --- a/lib/github_changelog_generator/generator/generator_processor.rb +++ b/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