Add license filter to discover-repos.rb #284
Conversation
Orillio
force-pushed
the
filter-licenses
branch
from
007bddb to
530e221
Compare
steps/discover-repos.rb
Outdated
| @@ -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 = [ | |||
There was a problem hiding this comment.
@Orillio we should provide this list from the command line. Now they are hardcoded, which makes it impossible to configure.
There was a problem hiding this comment.
@yegor256 I wanted to propose this as the issue for other students.
- 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.
steps/discover-repos.rb
Outdated
| licenses_to_filter = [ | ||
| { key: 'mit', name: 'MIT License' }, | ||
| { key: 'apache-2.0', name: 'Apache License 2.0' }, | ||
| { key: '0bsd', name: 'BSD Zero Clause License' } |
There was a problem hiding this comment.
@Orillio why do we need their names? I believe, just key will be enough.
There was a problem hiding this comment.
@yegor256 In case someone wanted to include license names inside the exported files (csv/tex), they may become handy.
I will remove them, if it causes issues.
There was a problem hiding this comment.
@Orillio it looks like information duplication at the moment
|
@Orillio please, read this: https://www.yegor256.com/2024/04/01/ping-me-please.html |
|
@yegor256 can you please check CI. Something strange is happening there |
steps/discover-repos.rb
Outdated
| @@ -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 = [ | |||
There was a problem hiding this comment.
|
@yegor256 please check new commit |
steps/discover-repos.rb
Outdated
| end | ||
| puts "Found #{json[:items].count} repositories in page ##{page}" | ||
| puts "Found #{found.count} repositories in page ##{page}" |
There was a problem hiding this comment.
@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.
steps/discover-repos.rb
Outdated
| @@ -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]}" | |||
There was a problem hiding this comment.
@Orillio why do we need their "names"? I suggest to rely only on the :key and ignore the name
steps/discover-repos.rb
Outdated
| else | ||
| github.search_repositories(query, per_page: size, page: page) | ||
| end | ||
| json[:items].each do |i| | ||
| next if i[:license].nil? || !licenses.include?(i[:license][:key]) |
There was a problem hiding this comment.
@Orillio I suggest to print a log line, when this happens
There was a problem hiding this comment.
@yegor256 I wanted to add this logic, but loop block is reached its maximum line length of 30.
I could have carry out this chunk of code to another function, but it wont allow me, because CI linter allows functions with length no more than 10 lines, which is highly unreasonable in my opinion.
found[i[:full_name]] = {
full_name: i[:full_name],
default_branch: i[:default_branch],
stars: i[:stargazers_count],
forks: i[:forks_count],
created_at: i[:created_at].iso8601,
size: i[:size],
open_issues_count: i[:open_issues_count],
description: i[:description],
topics: i[:topics]
}At this point, i dont know how to add new lines here without fully refactoring the code
There was a problem hiding this comment.
@yegor256 I somehow managed to overcome the issue. Please check the commit.
|
@Orillio thanks! |
Implemented filtration for repositories with licenses (MIT, Apache 2.0, BSD) as proposed in #275.
As room for improvement: