From 866c9f95d38ac04463054c4a19321997dc4eab3e Mon Sep 17 00:00:00 2001 From: Petr Korolev Date: Mon, 18 May 2015 09:26:36 +0300 Subject: [PATCH 1/8] print error more descriptive --- lib/github_changelog_generator/fetcher.rb | 49 ++++++++++++++--------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/lib/github_changelog_generator/fetcher.rb b/lib/github_changelog_generator/fetcher.rb index 54533e5..129992e 100644 --- a/lib/github_changelog_generator/fetcher.rb +++ b/lib/github_changelog_generator/fetcher.rb @@ -23,7 +23,7 @@ module GitHubChangelogGenerator @logger.formatter = proc do |_severity, _datetime, _progname, msg| "#{msg}\n" end - github_options = { per_page: PER_PAGE_NUMBER } + github_options = {per_page: PER_PAGE_NUMBER} github_options[:oauth_token] = @github_token unless @github_token.nil? github_options[:endpoint] = options[:github_endpoint] unless options[:github_endpoint].nil? github_options[:site] = options[:github_endpoint] unless options[:github_site].nil? @@ -60,30 +60,39 @@ module GitHubChangelogGenerator tags = [] begin - response = @github.repos.tags @options[:user], @options[:project] - page_i = 0 - count_pages = response.count_pages - response.each_page do |page| - page_i += PER_PAGE_NUMBER - print "Fetching tags... #{page_i}/#{count_pages * PER_PAGE_NUMBER}\r" - tags.concat(page) + github_fetch_tags(tags) + rescue Github::Error::Unauthorized => e + @logger.error e.body.red + @logger.error "Error: wrong github token provided".red + rescue Github::Error::Forbidden => e + @logger.warn e.body.red + unless @options[:token].nil? + @logger.warn GH_RATE_LIMIT_EXCEEDED_MSG.yellow end - print " \r" - - if tags.count == 0 - @logger.warn "Warning: Can't find any tags in repo.\ -Make sure, that you push tags to remote repo via 'git push --tags'".yellow - elsif @options[:verbose] - @logger.info "Found #{tags.count} tags" - end - - rescue - @logger.warn GH_RATE_LIMIT_EXCEEDED_MSG.yellow end tags end + def github_fetch_tags(tags) + response = @github.repos.tags @options[:user], @options[:project] + page_i = 0 + count_pages = response.count_pages + response.each_page do |page| + page_i += PER_PAGE_NUMBER + print "Fetching tags... #{page_i}/#{count_pages * PER_PAGE_NUMBER}\r" + tags.concat(page) + end + print " \r" + + if tags.count == 0 + @logger.warn "Warning: Can't find any tags in repo.\ +Make sure, that you push tags to remote repo via 'git push --tags'".yellow + elsif @options[:verbose] + @logger.info "Found #{tags.count} tags" + end + end + # This method fetch all closed issues and separate them to pull requests and pure issues # (pull request is kind of issue in term of GitHub) # @return [Tuple] with issues and pull requests @@ -108,6 +117,8 @@ Make sure, that you push tags to remote repo via 'git push --tags'".yellow break if @options[:max_issues] && issues.length >= @options[:max_issues] end rescue + puts e.message + puts e.backtrace.inspect @logger.warn GH_RATE_LIMIT_EXCEEDED_MSG.yellow end From 1ee1dfd50fb556d38d62982ebd4424a6828956ab Mon Sep 17 00:00:00 2001 From: Petr Korolev Date: Mon, 18 May 2015 15:04:15 +0300 Subject: [PATCH 2/8] wrap github methods in another method --- lib/github_changelog_generator/fetcher.rb | 42 +++++++++++------------ 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/lib/github_changelog_generator/fetcher.rb b/lib/github_changelog_generator/fetcher.rb index a076b8b..3ee31b3 100644 --- a/lib/github_changelog_generator/fetcher.rb +++ b/lib/github_changelog_generator/fetcher.rb @@ -9,8 +9,10 @@ module GitHubChangelogGenerator class Fetcher PER_PAGE_NUMBER = 30 CHANGELOG_GITHUB_TOKEN = "CHANGELOG_GITHUB_TOKEN" - GH_RATE_LIMIT_EXCEEDED_MSG = "Warning: GitHub API rate limit (5000 per hour) exceeded, change log may be " \ - "missing some issues. You can limit the number of issues fetched using the `--max-issues NUM` argument." + GH_RATE_LIMIT_EXCEEDED_MSG = "Warning: Can't finish operation: GitHub API rate limit exceeded, change log may be " \ + "missing some issues. You can limit the number of issues fetched using the `--max-issues NUM` argument." + NO_TOKEN_PROVIDED = "Warning: No token provided (-t option) and variable $CHANGELOG_GITHUB_TOKEN was not found. " \ + "This script can make only 50 requests to GitHub API per hour without token!" def initialize(options = {}) @options = options @@ -29,11 +31,7 @@ module GitHubChangelogGenerator github_options[:endpoint] = options[:github_endpoint] unless options[:github_endpoint].nil? github_options[:site] = options[:github_endpoint] unless options[:github_site].nil? - begin - @github = Github.new github_options - rescue - @logger.warn GH_RATE_LIMIT_EXCEEDED_MSG.yellow - end + @github = check_github_response{Github.new github_options} end # Returns GitHub token. First try to use variable, provided by --token option, @@ -44,8 +42,7 @@ module GitHubChangelogGenerator env_var = @options[:token] ? @options[:token] : (ENV.fetch CHANGELOG_GITHUB_TOKEN, nil) unless env_var - @logger.warn "Warning: No token provided (-t option) and variable $CHANGELOG_GITHUB_TOKEN was not found.".yellow - @logger.warn "This script can make only 50 requests to GitHub API per hour without token!".yellow + @logger.warn NO_TOKEN_PROVIDED.yellow end env_var @@ -60,21 +57,24 @@ module GitHubChangelogGenerator tags = [] - begin - github_fetch_tags(tags) - rescue Github::Error::Unauthorized => e - @logger.error e.body.red - @logger.error "Error: wrong github token provided".red - rescue Github::Error::Forbidden => e - @logger.warn e.body.red - unless @options[:token].nil? - @logger.warn GH_RATE_LIMIT_EXCEEDED_MSG.yellow - end - end + check_github_response { github_fetch_tags(tags) } tags end + def check_github_response + begin + value = yield + rescue Github::Error::Unauthorized => e + @logger.error e.body.red + abort "Error: wrong GitHub token" + rescue Github::Error::Forbidden => e + @logger.warn e.body.red + @logger.warn GH_RATE_LIMIT_EXCEEDED_MSG.yellow + end + value + end + def github_fetch_tags(tags) response = @github.repos.tags @options[:user], @options[:project] page_i = 0 @@ -118,8 +118,6 @@ Make sure, that you push tags to remote repo via 'git push --tags'".yellow break if @options[:max_issues] && issues.length >= @options[:max_issues] end rescue - puts e.message - puts e.backtrace.inspect @logger.warn GH_RATE_LIMIT_EXCEEDED_MSG.yellow end From e9cb010f09dfdb153526ac8061e87923b3a0f972 Mon Sep 17 00:00:00 2001 From: Petr Korolev Date: Mon, 18 May 2015 15:50:10 +0300 Subject: [PATCH 3/8] fix rubocop warnings --- lib/CHANGELOG.md | 2 ++ lib/github_changelog_generator/fetcher.rb | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/CHANGELOG.md b/lib/CHANGELOG.md index 745eb59..cfdcefc 100644 --- a/lib/CHANGELOG.md +++ b/lib/CHANGELOG.md @@ -6,6 +6,8 @@ **Merged pull requests:** +- this pr will be closed and shouldn't appear in changelog [\#7](https://github.com/skywinder/changelog_test/pull/7) ([skywinder](https://github.com/skywinder)) + - This PR SHOULD NOT appear in change log! [\#6](https://github.com/skywinder/changelog_test/pull/6) ([skywinder](https://github.com/skywinder)) - Add automatically generated change log file. [\#5](https://github.com/skywinder/changelog_test/pull/5) ([skywinder](https://github.com/skywinder)) diff --git a/lib/github_changelog_generator/fetcher.rb b/lib/github_changelog_generator/fetcher.rb index 3ee31b3..00fbb7a 100644 --- a/lib/github_changelog_generator/fetcher.rb +++ b/lib/github_changelog_generator/fetcher.rb @@ -26,12 +26,12 @@ module GitHubChangelogGenerator @project = @options[:project] @github_token = fetch_github_token @tag_times_hash = {} - github_options = {per_page: PER_PAGE_NUMBER} + github_options = { per_page: PER_PAGE_NUMBER } github_options[:oauth_token] = @github_token unless @github_token.nil? github_options[:endpoint] = options[:github_endpoint] unless options[:github_endpoint].nil? github_options[:site] = options[:github_endpoint] unless options[:github_site].nil? - @github = check_github_response{Github.new github_options} + @github = check_github_response { Github.new github_options } end # Returns GitHub token. First try to use variable, provided by --token option, From 9a3c06861522a4a5ce1990c349f69fb67da11d8d Mon Sep 17 00:00:00 2001 From: Petr Korolev Date: Mon, 18 May 2015 16:55:49 +0300 Subject: [PATCH 4/8] add test for error raising --- spec/unit/fetcher_spec.rb | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/spec/unit/fetcher_spec.rb b/spec/unit/fetcher_spec.rb index 709563f..6baf815 100644 --- a/spec/unit/fetcher_spec.rb +++ b/spec/unit/fetcher_spec.rb @@ -1,5 +1,16 @@ +VALID_TOKEN = "0123456789abcdef" +INVALID_TOKEN = "0000000000000000" + +DEFAULT_OPTIONS = { user: "skywinder", + project: "changelog_test" } + +def options_with_invalid_token + options = DEFAULT_OPTIONS + options[:token] = INVALID_TOKEN + options +end + describe GitHubChangelogGenerator::Fetcher do - VALID_TOKEN = "0123456789abcdef" before(:all) do @fetcher = GitHubChangelogGenerator::Fetcher.new end @@ -33,4 +44,16 @@ describe GitHubChangelogGenerator::Fetcher do it { is_expected.to eq(VALID_TOKEN) } end end + + describe "#github_fetch_tags" do + context "when wrong token provided" do + before do + options = options_with_invalid_token + @fetcher = GitHubChangelogGenerator::Fetcher.new(options) + end + it "should raise Unauthorized error" do + expect { @fetcher.github_fetch_tags [] }.to raise_error Github::Error::Unauthorized + end + end + end end From d0defc4c9b0679ea089a4e2fe1d03761cd6ac809 Mon Sep 17 00:00:00 2001 From: Petr Korolev Date: Tue, 19 May 2015 10:12:53 +0300 Subject: [PATCH 5/8] Prettify log output --- lib/github_changelog_generator.rb | 6 +--- lib/github_changelog_generator/fetcher.rb | 41 +++++++++++++---------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/lib/github_changelog_generator.rb b/lib/github_changelog_generator.rb index ea7576c..baa1520 100755 --- a/lib/github_changelog_generator.rb +++ b/lib/github_changelog_generator.rb @@ -149,10 +149,6 @@ module GitHubChangelogGenerator pr[:merged_at] = fetched_pr[:merged_at] pull_requests.delete(fetched_pr) } - - if @options[:verbose] - puts "Fetching merged dates: Done!" - end end # Include issues with labels, specified in :include_labels @@ -279,7 +275,7 @@ module GitHubChangelogGenerator threads.each(&:join) if @options[:verbose] - puts "Fetching tags dates: #{i} Done!" + puts "Fetching tags dates: #{i}" end end diff --git a/lib/github_changelog_generator/fetcher.rb b/lib/github_changelog_generator/fetcher.rb index 00fbb7a..1ecfbfc 100644 --- a/lib/github_changelog_generator/fetcher.rb +++ b/lib/github_changelog_generator/fetcher.rb @@ -81,15 +81,15 @@ module GitHubChangelogGenerator count_pages = response.count_pages response.each_page do |page| page_i += PER_PAGE_NUMBER - print "Fetching tags... #{page_i}/#{count_pages * PER_PAGE_NUMBER}\r" + print_in_same_line("Fetching tags... #{page_i}/#{count_pages * PER_PAGE_NUMBER}") tags.concat(page) end - print " \r" + print_empty_line if tags.count == 0 @logger.warn "Warning: Can't find any tags in repo.\ Make sure, that you push tags to remote repo via 'git push --tags'".yellow - elsif @options[:verbose] + else @logger.info "Found #{tags.count} tags" end end @@ -113,20 +113,17 @@ Make sure, that you push tags to remote repo via 'git push --tags'".yellow count_pages = response.count_pages response.each_page do |page| page_i += PER_PAGE_NUMBER - print "Fetching issues... #{page_i}/#{count_pages * PER_PAGE_NUMBER}\r" + print_in_same_line("Fetching issues... #{page_i}/#{count_pages * PER_PAGE_NUMBER}") issues.concat(page) break if @options[:max_issues] && issues.length >= @options[:max_issues] end + print_empty_line + @logger.info "Received issues: #{issues.count}" + rescue @logger.warn GH_RATE_LIMIT_EXCEEDED_MSG.yellow end - print " \r" - - if @options[:verbose] - @logger.info "Received issues: #{issues.count}" - end - # remove pull request from issues: issues.partition do |x| x[:pull_request].nil? @@ -140,20 +137,30 @@ Make sure, that you push tags to remote repo via 'git push --tags'".yellow begin response = @github.pull_requests.list @options[:user], @options[:project], state: "closed" page_i = 0 + count_pages = response.count_pages response.each_page do |page| page_i += PER_PAGE_NUMBER - count_pages = response.count_pages - print "Fetching merged dates... #{page_i}/#{count_pages * PER_PAGE_NUMBER}\r" + log_string = "Fetching merged dates... #{page_i}/#{count_pages * PER_PAGE_NUMBER}" + print_in_same_line(log_string) pull_requests.concat(page) end + print_empty_line rescue @logger.warn GH_RATE_LIMIT_EXCEEDED_MSG.yellow end - print " \r" + @logger.info "Fetching merged dates: #{pull_requests.count}" pull_requests end + def print_in_same_line(log_string) + print log_string + "\r" + end + + def print_empty_line + print_in_same_line(" ") + end + # Fetch event for all issues and add them to :events # @param [Array] issues # @return [Void] @@ -172,7 +179,7 @@ Make sure, that you push tags to remote repo via 'git push --tags'".yellow @logger.warn GH_RATE_LIMIT_EXCEEDED_MSG.yellow end issue[:events] = obj.body - print "Fetching events for issues and PR: #{i + 1}/#{issues.count}\r" + print_in_same_line("Fetching events for issues and PR: #{i + 1}/#{issues.count}") i += 1 end end @@ -181,11 +188,9 @@ Make sure, that you push tags to remote repo via 'git push --tags'".yellow end # to clear line from prev print - print " \r" + print_empty_line - if @options[:verbose] - @logger.info "Fetching events for issues and PR: #{i} Done!" - end + @logger.info "Fetching events for issues and PR: #{i}" end # Try to find tag date in local hash. From ec7c98758c04d946782c4cecda22aba5c9f93e4d Mon Sep 17 00:00:00 2001 From: Petr Korolev Date: Tue, 19 May 2015 11:47:56 +0300 Subject: [PATCH 6/8] fix #69 --- lib/CHANGELOG.md | 4 ---- lib/github_changelog_generator.rb | 18 ++++++++++++------ lib/github_changelog_generator/fetcher.rb | 8 ++++---- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/CHANGELOG.md b/lib/CHANGELOG.md index cfdcefc..a261d8f 100644 --- a/lib/CHANGELOG.md +++ b/lib/CHANGELOG.md @@ -6,10 +6,6 @@ **Merged pull requests:** -- this pr will be closed and shouldn't appear in changelog [\#7](https://github.com/skywinder/changelog_test/pull/7) ([skywinder](https://github.com/skywinder)) - -- This PR SHOULD NOT appear in change log! [\#6](https://github.com/skywinder/changelog_test/pull/6) ([skywinder](https://github.com/skywinder)) - - Add automatically generated change log file. [\#5](https://github.com/skywinder/changelog_test/pull/5) ([skywinder](https://github.com/skywinder)) ## [v0.0.3](https://github.com/skywinder/changelog_test/tree/v0.0.3) (2015-03-04) diff --git a/lib/github_changelog_generator.rb b/lib/github_changelog_generator.rb index baa1520..118b2e2 100755 --- a/lib/github_changelog_generator.rb +++ b/lib/github_changelog_generator.rb @@ -32,7 +32,8 @@ module GitHubChangelogGenerator # @all_tags = get_filtered_tags @all_tags = @fetcher.get_all_tags - @issues, @pull_requests = @fetcher.fetch_issues_and_pull_requests + # 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 @pull_requests = @options[:pulls] ? get_filtered_pull_requests : [] @@ -120,7 +121,7 @@ module GitHubChangelogGenerator # And exclude all from :exclude_labels array. # @return [Array] filtered PR's def get_filtered_pull_requests - fetch_merged_at_pull_requests + filter_merged_pull_requests filtered_pull_requests = include_issues_by_labels(@pull_requests) @@ -133,14 +134,15 @@ module GitHubChangelogGenerator filtered_pull_requests end - # This method fetch missing required attributes for pull requests + # 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 this date, not closed date. - def fetch_merged_at_pull_requests + # More correct to use merged date, rather than closed date. + def filter_merged_pull_requests if @options[:verbose] print "Fetching merged dates...\r" end - pull_requests = @fetcher.fetch_pull_requests + pull_requests = @fetcher.fetch_closed_pull_requests @pull_requests.each { |pr| fetched_pr = pull_requests.find { |fpr| @@ -149,6 +151,10 @@ module GitHubChangelogGenerator pr[:merged_at] = fetched_pr[:merged_at] pull_requests.delete(fetched_pr) } + + @pull_requests.select! do |pr| + !pr[:merged_at].nil? + end end # Include issues with labels, specified in :include_labels diff --git a/lib/github_changelog_generator/fetcher.rb b/lib/github_changelog_generator/fetcher.rb index 1ecfbfc..863a422 100644 --- a/lib/github_changelog_generator/fetcher.rb +++ b/lib/github_changelog_generator/fetcher.rb @@ -96,8 +96,8 @@ Make sure, that you push tags to remote repo via 'git push --tags'".yellow # This method fetch all closed issues and separate them to pull requests and pure issues # (pull request is kind of issue in term of GitHub) - # @return [Tuple] with issues and pull requests - def fetch_issues_and_pull_requests + # @return [Tuple] with (issues, pull-requests) + def fetch_closed_issues_and_pr if @options[:verbose] print "Fetching closed issues...\r" end @@ -124,7 +124,7 @@ Make sure, that you push tags to remote repo via 'git push --tags'".yellow @logger.warn GH_RATE_LIMIT_EXCEEDED_MSG.yellow end - # remove pull request from issues: + # separate arrays of issues and pull requests: issues.partition do |x| x[:pull_request].nil? end @@ -132,7 +132,7 @@ Make sure, that you push tags to remote repo via 'git push --tags'".yellow # Fetch all pull requests. We need them to detect :merged_at parameter # @return [Array] all pull requests - def fetch_pull_requests + def fetch_closed_pull_requests pull_requests = [] begin response = @github.pull_requests.list @options[:user], @options[:project], state: "closed" From 66152e59de1219ececb54324b7e12bc8ba92cfbf Mon Sep 17 00:00:00 2001 From: Petr Korolev Date: Tue, 19 May 2015 11:55:32 +0300 Subject: [PATCH 7/8] Fix #237 --- lib/github_changelog_generator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/github_changelog_generator.rb b/lib/github_changelog_generator.rb index 118b2e2..8a02b9a 100755 --- a/lib/github_changelog_generator.rb +++ b/lib/github_changelog_generator.rb @@ -219,7 +219,7 @@ module GitHubChangelogGenerator output_filename = "#{@options[:output]}" File.open(output_filename, "w") { |file| file.write(log) } puts "Done!" - puts "Generated log placed in #{`pwd`.strip!}/#{output_filename}" + puts "Generated log placed in #{Dir.pwd}/#{output_filename}" end # The full cycle of generation for whole project From a5d43b3d231da3acec1056f1457c398aaa6a065b Mon Sep 17 00:00:00 2001 From: Petr Korolev Date: Tue, 19 May 2015 12:19:26 +0300 Subject: [PATCH 8/8] Update gemspec to version 1.4.1 --- lib/github_changelog_generator/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/github_changelog_generator/version.rb b/lib/github_changelog_generator/version.rb index 8ec937b..65eb6cc 100644 --- a/lib/github_changelog_generator/version.rb +++ b/lib/github_changelog_generator/version.rb @@ -1,3 +1,3 @@ module GitHubChangelogGenerator - VERSION = "1.4.0" + VERSION = "1.4.1" end