Skip to content

Conversation

@Yegorov
Copy link
Contributor

@Yegorov Yegorov commented Oct 27, 2025

Need for zerocracy/judges-action#1160

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of repository size values for edge cases.
  • Tests

    • Added test coverage for empty repository and nil size scenarios.

@coderabbitai
Copy link

coderabbitai bot commented Oct 27, 2025

Walkthrough

The Fbe::FakeOctokit#repository method is refactored to use a case statement for computing repository size, mapping specific repository names to custom sizes: 'yegor256/empty-repo' yields 0, 'yegor256/nil-size-repo' yields nil, and others yield 470. Two new test methods validate this behavior.

Changes

Cohort / File(s) Summary
Core Logic Refactoring
lib/fbe/octo.rb
Refactored repository(name) method to replace inline conditional with case statement for size computation, adding explicit handling for 'yegor256/nil-size-repo' → nil and 'yegor256/empty-repo' → 0.
Test Coverage
test/fbe/test_octo.rb
Added test_fetches_fake_empty_repo and test_fetches_fake_nil_size_repo test methods to validate the new size-mapping behavior for empty and nil-size repositories.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • The refactoring follows a straightforward pattern with clear mappings and no complex logic
  • Test additions follow existing conventions
  • Changes are localized to a single method and its test coverage

Possibly related PRs

Poem

🐰 A case for each repo, neat and bright—
Empty ones return zero's light,
Nil-size ones handled just so,
Tests ensure we always know!
Rabbit hops through the code with glee! 🌙

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "add nil size for yegor256/nil-size-repo to Fbe#repository" accurately describes a primary change in the changeset. The PR modifies the repository size computation logic to handle the yegor256/nil-size-repo case by returning nil, adds a corresponding test case, and refactors the size assignment to use a case statement. The title is specific, clear, and directly related to the main objective referenced in the PR description (judges-action#1160), allowing reviewers to understand the intent without ambiguity.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8a0c0a9 and 13cf3ee.

📒 Files selected for processing (2)
  • lib/fbe/octo.rb (2 hunks)
  • test/fbe/test_octo.rb (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test/fbe/test_octo.rb (1)
lib/fbe/octo.rb (3)
  • octo (44-232)
  • o (288-290)
  • repository (613-659)
🔇 Additional comments (2)
lib/fbe/octo.rb (1)

618-623: LGTM! Clean refactoring with case statement.

The refactoring from an inline conditional to a case statement improves readability and maintainability. The new mapping for 'yegor256/nil-size-repo' correctly returns nil, which is appropriate for testing scenarios where repository size data is absent.

Also applies to: 637-637

test/fbe/test_octo.rb (1)

340-350: LGTM! Well-structured test coverage.

The two new test methods properly validate the special-case repository size mappings. The use of .then with a block is clean and idiomatic, and the assertions correctly verify both the zero-size and nil-size scenarios.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Yegorov
Copy link
Contributor Author

Yegorov commented Oct 27, 2025

@yegor256 merge this PR, please.

size =
case name
when 'yegor256/empty-repo' then 0
when 'yegor256/nil-size-repo' then nil
Copy link
Member

Choose a reason for hiding this comment

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

@Yegorov why this may happen? It looks like a bug in GitHub API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yegor256 Yes, it looks like a bug, but it could also be something with the token's access rights.

Copy link
Member

Choose a reason for hiding this comment

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

@Yegorov maybe we should fix the root cause: access rights problem. This change looks like a simulation of something that never happens in reality.

Copy link
Contributor Author

@Yegorov Yegorov Oct 28, 2025

Choose a reason for hiding this comment

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

@yegor256 Unfortunately, I can't help here, as I don't have the full context of the situation.
Can you make a request to view information about the repository with this token?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants