The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- Status changed to Needs review
almost 2 years ago 5:50am 31 January 2023 - 🇮🇳India gauravvvv Delhi, India
Fixed CCF and re-rolled patch for 10.1.x. Attached interdiff for same. please review
The last submitted patch, 12: 3216767-12.patch, failed testing. View results →
- last update
over 1 year ago 29,388 pass, 1 fail - last update
over 1 year ago 29,389 pass - 🇮🇳India ranjith_kumar_k_u Kerala
I have removed unrelated code changes on the previous patch and added tests, please review
The last submitted patch, 14: 3216767-14-test-only.patch, failed testing. View results →
- Status changed to Needs work
over 1 year ago 10:00pm 12 May 2023 - 🇺🇸United States smustgrave
What happens if NULL is passed in as min?
Was there not already tests checking for max_depth?
Change record will be required for the new parameter.
Not sure if a trigger_error will be needed here either?
- Status changed to Needs review
over 1 year ago 10:21am 19 May 2023 - 🇮🇳India ranjith_kumar_k_u Kerala
The bookTreeAllData() internally uses the doBookTreeBuild() function, so if we do not provide the $min_depth then doBookTreeBuild() will consider min_depth as 1.
There were no tests for the bookTreeAllData() function.
Created a change record - https://www.drupal.org/node/3361444 →
- Status changed to Needs work
over 1 year ago 7:47pm 19 May 2023 - 🇺🇸United States smustgrave
Would be nice for issue summary to be cleaned up some.
Tagging for subsystem review that if min_depth is greater then max_depth some log or error should be trigged I think.
- 🇺🇸United States pwolanin
Conceptually seems fine - maybe just hard-coded that way to 1 because it was in prior versions, or due to lack of a use case.
- Issue was unassigned.
- 🇳🇿New Zealand quietone
Un-assigning per Assigning ownership of a Drupal core issue → .
- Merge request !9Issue #3216767 by mhmhartman, ranjith_kumar_k_u: Book module: add min_depth (starting level) → (Merged) created by smustgrave
-
smustgrave →
committed f2c4bfe6 on 2.0.x
Issue #3216767 by mhmhartman, ranjith_kumar_k_u: Book module: add...
-
smustgrave →
committed f2c4bfe6 on 2.0.x
- Status changed to Fixed
7 months ago 7:15pm 18 June 2024 - 🇺🇸United States smustgrave
This was pretty close way back when so going to merge now in contrib.
Automatically closed - issue fixed for 2 weeks with no activity.