-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#186 Workaround to find more than 1000 repositories #378
Conversation
|
||
opts = Slop.parse do |o| | ||
o.string '--token', 'GitHub access token', default: '' | ||
o.boolean '--dry', 'Make no round-trips to GitHub API (for testing)', default: false | ||
o.integer '--total', 'Total number of repos to take from GitHub', required: true | ||
o.integer '--pause', 'How many seconds to sleep between API calls', default: 10 | ||
o.integer '--page-size', 'Number of repos to fetch in one API call', default: 100 | ||
o.integer '--min-stars', 'Minimum GitHub stars in each repo', default: max |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lueFlake what is the point of this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yegor256 why would we need to keep "max" variable? It is used only once here and i dont understand logic behind it in this line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or are you talking about refactoring of the year loop?
@lueFlake looks good, but there are some failures in the build |
@yegor256 yep, i did something wrong while refactoring, now fixed |
@yegor256 fixed issues with build, but it fails on |
steps/discover-repos.rb
Outdated
def process_year(year, github, context) | ||
query = build_query(year, context[:opts]) | ||
puts "Querying for repositories created in #{year}..." | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lueFlake what's the point of this empty line? https://www.yegor256.com/2014/11/03/empty-line-code-smell.html
steps/discover-repos.rb
Outdated
page = 0 | ||
loop do | ||
break if context[:found].count >= context[:opts][:total] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lueFlake better avoid them (empty lines)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yegor256 removed blank lines, forgot to remove them after debug, my bad
@lueFlake thanks! |
resolves #186