docs(changelog): merge the split Database section and record the redirect fix - #147
Merged
Merged
Conversation
…rect fix The 2.7.3 block carried two "Database:" headings, because XOOPS#143's entry was added at the top of the section rather than beside the existing one from XOOPS#130. They are one section now, and XOOPS#143 gained the PR reference it was missing. XOOPS#145 had no curated entry at all -- it landed after the section was written -- so the login redirect fix appears under a new "Redirects:" heading, with the detail the generated one-liner cannot carry: why a URL carrying an HTML entity hid the fault for exactly one hop, and what made it start accumulating.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideCleans up the curated 2.7.3 Beta 1 changelog notes by consolidating duplicate Database sections, adding the missing PR reference, and adding a curated entry for the login redirect fix. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="docs/changelog.270.txt" line_range="42" />
<code_context>
- keep the speculative unserialize() probe out of the error log (mamba) in #130
Database:
+- return the documented failure value instead of fataling when a result-set method is handed something that is not one. A failed query returns false and a mis-wired caller can pass the SQL string itself; PHP 8 turns both into an uncaught TypeError inside mysqli, so one bad call from one module blanked the whole page. Observed on xoops.org from modules/core/brokenfile.php. The misuse is still reported, so the caller's bug stays visible (mamba) in #143
- name the handler and table on query errors, and fail fast when the table is unset, so a failure identifies itself instead of surfacing as an anonymous error (mamba) in #130
</code_context>
<issue_to_address>
**suggestion (typo):** Consider rephrasing “fataling” to a more standard construction.
“Instead of fataling” is non-standard English. Consider alternatives like “instead of causing a fatal error” or “instead of failing fatally” for clearer wording.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| - keep the speculative unserialize() probe out of the error log (mamba) in #130 | ||
|
|
||
| Database: | ||
| - return the documented failure value instead of fataling when a result-set method is handed something that is not one. A failed query returns false and a mis-wired caller can pass the SQL string itself; PHP 8 turns both into an uncaught TypeError inside mysqli, so one bad call from one module blanked the whole page. Observed on xoops.org from modules/core/brokenfile.php. The misuse is still reported, so the caller's bug stays visible (mamba) in #143 |
Contributor
There was a problem hiding this comment.
suggestion (typo): Consider rephrasing “fataling” to a more standard construction.
“Instead of fataling” is non-standard English. Consider alternatives like “instead of causing a fatal error” or “instead of failing fatally” for clearer wording.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #147 +/- ##
============================================
+ Coverage 18.96% 19.29% +0.32%
- Complexity 8091 8227 +136
============================================
Files 670 672 +2
Lines 43925 44265 +340
============================================
+ Hits 8332 8539 +207
- Misses 35593 35726 +133 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Housekeeping on the curated 2.7.3 Beta 1 notes, found while writing the release announcement.
Changes
Two
Database:headings merged. #143's entry was added at the top of the 2.7.3 block rather than beside the existing entry from #130, so the section appeared twice. They are now one section, and #143's entry gained the PR reference it was missing.#145 had no curated entry. It landed after the section was written, so the login redirect fix now appears under a new
Redirects:heading — with the detail the generated one-liner cannot carry: why a URL holding an HTML entity hid the fault for exactly one hop, and what made it start accumulating layers.Note
Unrelated to this PR, but worth recording: the duplicate
v2.7.3_Beta1tag has been deleted, leaving onlyv2.7.3-Beta1. The auto-generated changelog PR (#146) was produced while both tags pointed at the same commit and picked the underscore form, so it needs regenerating before merge.Summary by Sourcery
Tidy up the curated 2.7.3 Beta 1 changelog entries for database and redirect changes.
Documentation:
Summary by CodeRabbit
Bug Fixes
Documentation