Skip to content

Conversation

@nattywebdev
Copy link

@nattywebdev nattywebdev commented Nov 30, 2023

Fixes #2

Replace unsupported create_function and fix consequent issue related to sorting files and folders.

Replace unsupported create_function and fix consequent issue related to sorting.
@argiepiano
Copy link

@nattywebdev I added the magic words "Fixes #.." in the first comment in this PR. This links this PR to issue #2, where the problem, was reported. This is commonly done (you can try it on the next PR you send!)

So, when this PR is merged, that issue will be closed automatically. @biolithic has not responded, so after two weeks, the @backdrop-contrib/bug-squad can merge and do a new release. Thanks!

spaces instead of tabs

Co-authored-by: Greg Netsas <[email protected]>
@klonos
Copy link
Member

klonos commented Dec 1, 2023

@argiepiano thank you for trying to stick to the rules, however biolithic is known to have moved away from the Backdrop community for some years now. His GitHub profile also shows very scarce to zero activity for the past 3 years (0-2 contributions/year) and none of it in the Backdrop space.

What I'm trying to say is that if this problem is causing grief to the community, we can rush things in this case. Pinging other members of the @backdrop-contrib/bug-squad to get some consensus on this.

array item to end with comma

Co-authored-by: Greg Netsas <[email protected]>
@nattywebdev
Copy link
Author

@klonos Thank you for your comments. I've committed the two suggested items - do I need to do anything else?

@klonos
Copy link
Member

klonos commented Dec 1, 2023

@nattywebdev I've added a couple of more code suggestion re coding standards (see https://docs.backdropcms.org/php-standards for details).

Also, there is one line that needs to be changed that GitHub does not allow me to provide a suggestion for: line 129 'data' => _filebrowser_thumbnails_generate($node, $data), also needs to have its indentation reduced by 2 spaces.

nattywebdev and others added 3 commits December 1, 2023 14:29
additional space before conditional :

Co-authored-by: Greg Netsas <[email protected]>
removed unnecessary space

Co-authored-by: Greg Netsas <[email protected]>
Corrected code format issues spotted by @klonos
@nattywebdev
Copy link
Author

@klonos I think I've addressed all those issues - thanks for the feedback!

@klonos
Copy link
Member

klonos commented Dec 1, 2023

Thank you @nattywebdev 👍🏼 ...yes, the code looks good now from a condign standards perspective - I haven't tested whether this actually fixes the issue or not, nor whether the code is causing any other regressions.

So to answer your question re what else remains to be done:

  • Testing: you need someone else other than you to test this code and add a comment in PHP compability: Function create_function() is deprecated #2 stating that they have tested it and it works as expected
  • Code review: I will add a comment to say that code looks OK
  • Wait: unless the rest of the @backdrop-contrib/bug-squad members agree that we can make an exception this time, you need to give the current maintainer a couple of weeks time to respond and action (either provide feedback and request code changes, or accept this change and merge it). If enough time has passed and there is no response from the person/people currently listed as maintainers of the project, then ping us (the Bug Squad group) here or in Zulip to action this.
  • Consider becoming a maintainer of this project (totally optional - no pressure)

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.

PHP compability: Function create_function() is deprecated

3 participants