Skip to content
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.

Sign up for GitHub

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

Add license filter to discover-repos.rb #284

Merged
merged 5 commits into from
Apr 21, 2024
Merged

Add license filter to discover-repos.rb #284

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b81918e
add license filter to discover-repos.rb
Orillio Apr 18, 2024
530e221
fix linter
Orillio Apr 18, 2024
9226214
remove redundant key
Orillio Apr 19, 2024
ccac1bd
refactor variable name
Orillio Apr 20, 2024
78a50a7
add skipping repo log
Orillio Apr 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 30 additions & 8 deletions steps/discover-repos.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@
raise 'Can only retrieve up to 1000 repos' if opts[:total] > max

size = [opts[:page_size], opts[:total]].min
licenses_to_filter = [
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Orillio we should provide this list from the command line. Now they are hardcoded, which makes it impossible to configure.

Copy link
Contributor Author

@Orillio Orillio Apr 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yegor256 I wanted to propose this as the issue for other students.

  1. Remove hardcoded licenses and let user choose desired licenses to be filtered

Since there are not many (easily solvable) issues in this repo right now.
As you said, the course isn't about solving tasks, but about open source collaboration/discussions.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Orillio check this out: https://www.yegor256.com/2015/01/12/compound-name-is-code-smell.html

'mit',
'apache-2.0',
'0bsd'
]

github = Octokit::Client.new
unless opts[:token].empty?
Expand All @@ -69,17 +74,34 @@
'NOT',
'android'
].join(' ')
loop do
if page * size > max
puts "Can't go to page ##{page}, since it will be over #{max}"
break

def mock_array(size, licenses_to_filter)
Array.new(size) do
{
full_name: "foo/#{Random.hex(5)}",
created_at: Time.now,
license: { key: licenses_to_filter.sample(1)[0] }
}
end
json = if opts[:dry]
{ items: page > 100 ? [] : Array.new(size) { { full_name: "foo/#{Random.hex(5)}", created_at: Time.now } } }
end

def mock_reps(page, size, licenses_to_filter)
{
items: if page > 100 then []
else
mock_array(size, licenses_to_filter)
end
}
end

loop do
break if page * size > max
json = if opts[:dry] then mock_reps(page, size, licenses_to_filter)
else
github.search_repositories(query, per_page: size, page: page)
end
json[:items].each do |i|
next if i[:license].nil? || !licenses_to_filter.include?(i[:license][:key])
found[i[:full_name]] = {
full_name: i[:full_name],
default_branch: i[:default_branch],
Expand All @@ -92,9 +114,9 @@
topics: i[:topics]
}
puts "Found #{i[:full_name].inspect} GitHub repo ##{found.count} \
(#{i[:forks_count]} forks, #{i[:stargazers_count]} stars)"
(#{i[:forks_count]} forks, #{i[:stargazers_count]} stars) with license: #{i[:license][:name]}"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Orillio why do we need their "names"? I suggest to rely only on the :key and ignore the name

end
puts "Found #{json[:items].count} repositories in page ##{page}"
puts "Found #{found.count} repositories in page ##{page}"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Orillio now, we will print:

10 repositories in page #1 
20 repositories in page #2 
30 repositories in page #2 
40 repositories in page #2 
...

which is obviously wrong.

break if found.count >= opts[:total]
puts "Let's sleep for #{opts[:pause]} seconds to cool off GitHub API \
(already found #{found.count} repos, need #{opts[:total]})..."
Expand Down
Loading