backdrop-contrib / filebrowser

The FileBrowser module displays contents of a given directory as a node on your Backdrop site - do not use together with Filebrowser AJAX module
GNU General Public License v2.0
0 stars 3 forks source link

issue 2 - replace create_function and fix sorting #6

Open nattywebdev opened 10 months ago

nattywebdev commented 10 months ago

Fixes #2

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

argiepiano commented 10 months ago

@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!

klonos commented 10 months ago

@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.

nattywebdev commented 10 months ago

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

klonos commented 10 months ago

@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 commented 10 months ago

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

klonos commented 10 months ago

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: