Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
In 9.x we deprecated some code in book.module, it's time to remove it.
Steps to reproduce
Proposed resolution
Remove the code, ensure no usages remain.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3263391-2.patch | 15.18 KB | longwave |
Comments
Comment #2
longwaveComment #3
tmaiochi CreditAttribution: tmaiochi at CI&T commentedI'll review this
Comment #4
tmaiochi CreditAttribution: tmaiochi at CI&T commentedSteps performed:
(1) Installed module
(2) Reproduced the issue.
(3) Applied patch.
(4) Code review on changes.
(5) Tested again with patch, issue resolved.
The patch removed all deprecated code from Book module, and everything still working as far as I can test it
Comment #7
catch@tmaiochi this issue is removing deprecated code that doesn't run when book module is installed, it would only run if another module depended on it. So, there's no need for manual testing, and no bug to reproduce - posting those as steps actually makes it look like less review happened rather than more.
To get contribution credit for reviewing an issue like this, there are really only a couple of things that need doing:
- A visual review of the patch just to check it's only removing code that's deprecated, and that the code is deprecated for removal in Drupal 10 (it's not impossible to have code deprecated for removal in Drupal 11).
- A quick grep/look over of book module to make sure there's no deprecated code left in it once the patch is applied. For example if a test is removed that uses a fixture, it's possible the fixture could be removed if it's not used by any other tests.
So, I haven't assigned contribution credit for this issue, but hopefully this will help with reviewing similar issues in the future.
Patch looks good and the test fail is unrelated.
Committed f431117 and pushed to 10.0.x. Thanks!