joomla-cms icon indicating copy to clipboard operation
joomla-cms copied to clipboard

[5.3] Deprecations: Changes we can make with 5.3 for the actionlog component (Backend)

Open rdeutz opened this issue 1 year ago • 8 comments

Summary of Changes

Implementing changes to our codebase regarding depreciated code. Done by component and splited into back and frontend for easier testing.

Testing Instructions

Test the backend for the action log component. Test also other componets e.g. user who add information to the action log.

Actual result BEFORE applying this Pull Request

Anything works

Expected result AFTER applying this Pull Request

Anything works

rdeutz avatar Oct 15 '24 16:10 rdeutz

Can you please update the PR title so that it is clearer what is being changed here.

brianteeman avatar Oct 15 '24 16:10 brianteeman

@brianteeman what's the issue when removing (C) ? the @copyright already says that it's about the copyright isn't it? as alternative we could use © can't we? just asking

HLeithner avatar Oct 16 '24 18:10 HLeithner

@brianteeman what's the issue when removing (C) ? the @copyright already says that it's about the copyright isn't it? as alternative we could use © can't we? just asking

  1. We should only use one form of copyright statement throughout Joomla
  2. In many countries - including the USA - the @copyright "element" does not match the legal requirements of a copyright statement
  3. Using © instead of (c) is ok but we must be consistent

I can provide more legal references if required

brianteeman avatar Oct 16 '24 18:10 brianteeman

@brianteeman using "Copyright" as word should also be ok, right? But I am looking what is the best way in terms of IDE configuration.

rdeutz avatar Oct 17 '24 07:10 rdeutz

@brianteeman using "Copyright" as word should also be ok, right? But I am looking what is the best way in terms of IDE configuration.

In the USA it would be ok - not sure about elsewhere. We should be consistent though.

brianteeman avatar Oct 17 '24 09:10 brianteeman

I did it again on another way, so anything should be fine now and the PR can be tested

rdeutz avatar Oct 17 '24 09:10 rdeutz

I am confused about this codestyle as it doesnt appear to be consistent

        if (!empty($extension)) {
            $extension .= '%';
            $query
                ->where($db->quoteName('a.extension') . ' LIKE :extension')
                ->bind(':extension', $extension);
        }
                $query->where(
                    $db->quoteName('a.log_date') . ' BETWEEN :dstart AND :dnow',
                );
                $query->bind(':dstart', $dStart);
                $query->bind(':dnow', $dNow);

In the second one shouldnt the two bind queries be part of the first query - so something like

                $query
                ->where($db->quoteName('a.log_date') . ' BETWEEN :dstart AND :dnow')
                ->bind(':dstart', $dStart);
                ->bind(':dnow', $dNow);

brianteeman avatar Oct 17 '24 10:10 brianteeman

I missed the whole file.

About the code style for multiline chaining. Honestly I am not 100% sure what it better readable. The reformat applies when you have something like

$var->action1()
  ->action2()
  ->action3()

then it is converted into

$var
  ->action1()
  ->action2()
  ->action3()

It might be better to let it as it is, the difference is not so big

rdeutz avatar Oct 17 '24 10:10 rdeutz

This pull request has been automatically rebased to 6.0-dev.

HLeithner avatar Mar 04 '25 17:03 HLeithner