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

#46: ignore archived repositories for Fbe#unmask_repos #49

Merged
merged 1 commit into from
Aug 2, 2024
Merged

#46: ignore archived repositories for Fbe#unmask_repos #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion lib/fbe/octo.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ def repository(name)
has_pages: false,
has_discussions: false,
forks_count: 0,
archived: false,
archived: name == 'zerocracy/datum',
disabled: false,
open_issues_count: 6,
license: { key: 'mit', name: 'MIT License' },
Expand Down
4 changes: 3 additions & 1 deletion lib/fbe/unmask_repos.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,23 @@ def self.mask_to_regex(mask)

def self.unmask_repos(options: $options, global: $global, loog: $loog)
repos = []
octo = Fbe.octo(loog:, global:, options:)
masks = (options.repositories || '').split(',')
masks.reject { |m| m.start_with?('-') }.each do |mask|
unless mask.include?('*')
repos << mask
next
end
re = Fbe.mask_to_regex(mask)
Fbe.octo(loog:, global:, options:).repositories(mask.split('/')[0]).each do |r|
octo.repositories(mask.split('/')[0]).each do |r|
repos << r[:full_name] if re.match?(r[:full_name])
end
end
masks.select { |m| m.start_with?('-') }.each do |mask|
re = Fbe.mask_to_regex(mask[1..])
repos.reject! { |r| re.match?(r) }
end
repos.reject! { |repo| octo.repository(repo)[:archived] }
Copy link
Member

Choose a reason for hiding this comment

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

@Yegorov you are making an extra HTTP request to GitHub API. Maybe it's better to use the JSON already retrieved a few lines earlier, in line 48?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe it's better to use the JSON already retrieved a few lines earlier, in line 48?

@yegor256 I examined this case, but there may be a situation when an added repository without a mask is already in the archive

See this code: we added repos without make request to GitHub API:

      unless mask.include?('*')
        repos << mask
        next
      end

Therefore, we should do an archive test at the very end

raise "No repos found matching: #{options.repositories}" if repos.empty?
loog.debug("Scanning #{repos.size} repositories: #{repos.join(', ')}...")
repos
Expand Down
11 changes: 7 additions & 4 deletions test/fbe/test_unmask_repos.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,22 +37,25 @@ def test_simple_use
opts = Judges::Options.new(
{
'testing' => true,
'repositories' => 'yegor256/tacit,zerocracy/*,-zerocracy/judges-action'
'repositories' => 'yegor256/tacit,zerocracy/*,-zerocracy/judges-action,zerocracy/datum'
}
)
assert(Fbe.unmask_repos(options: opts, global: {}, loog: Loog::NULL).size.positive?)
list = Fbe.unmask_repos(options: opts, global: {}, loog: Loog::NULL)
assert(list.size.positive?)
refute_includes(list, 'zerocracy/datum')
end

def test_live_usage
skip('Run it only manually, since it touches GitHub API')
Judges::Options.new(
opts = Judges::Options.new(
{
'repositories' => 'zerocracy/*,-zerocracy/judges-action'
'repositories' => 'zerocracy/*,-zerocracy/judges-action,zerocracy/datum'
}
)
list = Fbe.unmask_repos(options: opts, global: {}, loog: Loog::NULL)
assert(list.size.positive?)
assert(list.include?('zerocracy/pages-action'))
assert(!list.include?('zerocracy/judges-action'))
refute_includes(list, 'zerocracy/datum')
end
end