This repository was archived by the owner on Sep 23, 2025. It is now read-only.
Merge from master #665
Merged
Merge from master #665
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Merge pull request #656 from iKostanOrg/master
suggestion (testing): Test cases for invalid sizes are missing. It would be beneficial to add tests for invalid sizes like rectangular or non-square matrices, such as 2x3 or 3x4. This would ensure more comprehensive test coverage. Depending on where you insert these changes in your parameterized test list (e.g., near the top or in the middle), ensure that the commas and brackets match the existing code structure.
Merge from master
Merge pull request #658 from iKostanOrg/master
Merge from master
Merge from master
Merge pull request #660 from iKostanOrg/master
Merge from master
Merge pull request #662 from iKostanOrg/master
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more Footnotes
|
Contributor
Reviewer's Guide by SourceryThis pull request includes several key updates: test refactoring to use parameterized tests for improved readability and coverage, standardization of GitHub Actions workflows, and updates to the README file to accurately reflect solution availability. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @ikostan - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding type hints to function signatures for better readability and maintainability.
- It looks like you've added a lot of tests using parameterized, which is great, but consider refactoring common setup code into setUp methods to avoid repetition.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟡 Security: 1 issue found
- 🟡 Testing: 2 issues found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines 4 to +7
| on: # yamllint disable-line rule:truthy | ||
| push: | ||
| branches: | ||
| - 'kyu3' | ||
| - kyu3 |
Contributor
There was a problem hiding this comment.
🚨 suggestion (security): Explicit permissions reduce token scope.
Adding an explicit permissions block with 'contents: read' and 'pull-requests: read' helps enforce the principle of least privilege.
Suggested change
| on: # yamllint disable-line rule:truthy | |
| push: | |
| branches: | |
| - 'kyu3' | |
| - kyu3 | |
| on: # yamllint disable-line rule:truthy | |
| push: | |
| branches: | |
| - kyu3 | |
| permissions: | |
| contents: read | |
| pull-requests: read |
Comment on lines +57 to +66
| ([[1, 0, 0, 0, 0, 1, 1, 0, 0, 0], | ||
| [1, 0, 1, 0, 0, 0, 0, 0, 1, 0], | ||
| [1, 0, 1, 0, 1, 1, 1, 0, 1, 0], | ||
| [1, 0, 0, 0, 0, 0, 0, 0, 0, 0], | ||
| [0, 0, 0, 0, 0, 0, 0, 0, 1, 0], | ||
| [0, 0, 0, 0, 1, 1, 1, 0, 0, 0], | ||
| [0, 0, 0, 0, 0, 0, 0, 0, 1, 0], | ||
| [0, 0, 0, 1, 0, 0, 0, 0, 0, 0], | ||
| [0, 0, 0, 0, 0, 0, 0, 1, 0, 0], | ||
| [0, 0, 0, 0, 0, 0, 0, 0, 0, 0]], |
Contributor
There was a problem hiding this comment.
suggestion (testing): Suggest adding more test cases for edge cases and invalid inputs.
Consider adding tests for scenarios like all zeros, all ones, invalid ship sizes, ships placed outside the grid, overlapping ships, and other non-standard inputs to ensure comprehensive coverage.
| " \\| \\|", | ||
| "X-----+------+", | ||
| " \\| ", | ||
| " X "]]]) |
Contributor
There was a problem hiding this comment.
suggestion (testing): Add tests for invalid grid formats.
The tests currently focus on valid paths. Consider adding tests with invalid characters, non-rectangular grids, or grids with multiple start/end points to ensure the Walker class handles these scenarios gracefully.
ikostan added a commit that referenced this pull request Feb 24, 2025
Merge pull request #665 from iKostanOrg/master
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Refactor and enhance test suites for several katas, improving readability, maintainability, and test coverage by using parameterized tests and adding more test cases. Also, improve test logging and reporting with allure steps for better traceability.
Enhancements:
Tests: