Fix: Resolved string bounding bug in UI (Fixes #1234)#7499
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7499 +/- ##
============================================
+ Coverage 79.91% 80.23% +0.31%
- Complexity 7353 7357 +4
============================================
Files 811 810 -1
Lines 23879 23789 -90
Branches 4705 4679 -26
============================================
+ Hits 19082 19086 +4
+ Misses 4036 3944 -92
+ Partials 761 759 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Hi @Abhi13shek , thanks for the contribution! I reviewed the diff and have a few observations:
The PR title says "Fixes #1234", but issue #1234 ("Can I add convex hull algorithms, some missing graph algos and hashing?") is a 2020 feature request that was auto-closed as stale in 2021. It has no connection to the changes in this PR — no convex hull, graph algorithm, or hashing code is touched here. Could you confirm the correct issue number this PR is meant to address? This PR mixes two separate concerns: NQueens.java: replacing System.out.println with Logger.info Per the contributing guidelines, it's usually cleaner to split unrelated changes into separate PRs — this makes review and rollback easier if something needs to be reverted later. Codecov flags 4 missing lines in NQueens.java (63.6% patch coverage). It might be worth adding a test case that covers the logger branches (the "no solution found" and "arrangement found" paths) to close that gap. In NQueens.java, LOGGER.info("") (replacing the old blank System.out.println()) doesn't really add value — loggers print structured lines (timestamp/level), so an empty call just creates a noisy blank entry rather than visual spacing. Might be worth removing it. |
DenizAltunkapan
left a comment
There was a problem hiding this comment.
I agree with @prashantpiyush1111 — the same points block this for me. Short summary of what to change:
- Fix the issue reference. "Fixes #1234" points to an unrelated closed issue. Remove it or use the correct number.
- Split the PR. NQueens (logger) and StackPostfixNotation (Deque) are unrelated and should be two PRs.
- Remove
LOGGER.info(""). An empty log line only adds noise. - Add a test for the NQueens logger branches to cover the 4 missing lines.
Requesting changes until these are addressed.
clang-format -i --style=file path/to/your/file.java