Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #5137: Initialise the path_info return array. #3673

Merged
merged 1 commit into from
Aug 12, 2021

Conversation

jayelless
Copy link
Contributor

@jayelless jayelless commented Jul 23, 2021

@backdrop-ci
Copy link
Collaborator

Related to: backdrop/backdrop-issues#5137

@jayelless
Copy link
Contributor Author

These fails appear unrelated to the change. What do they mean?

@jayelless
Copy link
Contributor Author

I am unimpressed. The exact same commit that failed originally has now passed all tests. I have just wasted several hours :(

@indigoxela
Copy link
Member

I am unimpressed. The exact same commit that failed originally has now passed all tests.

Yes, that can be frustrating. We're increasingly having trouble with Zen.CI and are in the middle of the process to switch to GitHub Actions. But that's unrelated here.

Sorry for your inconvenience with our "cheap" infrastructure - please keep in mind: Backdrop is an Open Source project.

@@ -596,7 +596,7 @@ function path_get_info() {
$all_info = backdrop_static(__FUNCTION__);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be:

  $all_info = &backdrop_static(__FUNCTION__);

To use this design pattern, you need to use a reference assignment to backdrop_static() (see the &); otherwise, you don't get a proper reference to the static stuff inside.

There are several places in Backdrop core that the & is missing, but that will be a separate issue (to come).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that it is better to leave this PR as is, to address the identified code clean-up changes, and to implement the changes required for issue backdrop/backdrop-issues#5142 in a new PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following further consideration, I have now suggested an improved code change that includes that "&" character as required. Refer to backdrop/backdrop-issues#5137 (comment)

@@ -593,10 +593,9 @@ function path_config_info() {
* Get all path information from modules implementing hook_path_info().
*/
function path_get_info() {
$all_info = backdrop_static(__FUNCTION__);
$all_info = backdrop_static(__FUNCTION__, array());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jayelless as @bugfolder skipped that function in his ampersand PR, you'd have to add it here.

&backdrop_static(

@dragonbot
Copy link
Collaborator

Tugboat has finished building a preview for this pull request!

Website: https://pr3673-youeifhhq9hyeegmzvjogrqpyydkfvhr.tugboat.qa/
Username: admin
Password: 2086217d016e

@quicksketch quicksketch merged commit d71c205 into backdrop:1.x Aug 12, 2021
@quicksketch
Copy link
Member

Thanks! Merged into 1.x and 1.19.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fixed: Initialize the return array in path_get_info()
6 participants