akeeba / fof

Rapid Application Development framework for Joomla!™ 3 and 4
0 stars 0 forks source link

minor changes to comply with JED checker #663

Closed go-source closed 6 years ago

go-source commented 6 years ago

Nick, the reason is that I've packed my component with FOF3 and did not pass JED checker. If you choose to reject this pull request, please comment the code so that newbies like myself understand. Summary of JED checker - Compatibility Issues:

001 /pkg_gscrm_1.0.0/lib_fof30/fof/Input/Input.php in line: 98 (not changed)

002 /pkg_gscrm_1.0.0/lib_fof30/fof/Input/Input.php in line: 100 (changed to Jinput)

JRequest is deprecated, you should use JFactory::getApplication()->input;: please note that your have @codeCoverageIgnoreStart on line 97

003 /pkg_gscrm_1.0.0/lib_fof30/fof/Utils/InstallScript.php in line: 959

004 /pkg_gscrm_1.0.0/lib_fof30/fof/Utils/InstallScript.php in line: 996

JError is deprecated, you should use JFactory::getApplication()->enqueueMessage();:

4 Warning - (I did not change - how could I mess up superglobals!)

001 /pkg_gscrm_1.0.0/lib_fof30/fof/Input/Input.php in line: 31

Use of superglobals is strongly discouraged:

002 /pkg_gscrm_1.0.0/lib_fof30/fof/Input/Input.php in line: 34

Use of superglobals is strongly discouraged:

003 /pkg_gscrm_1.0.0/lib_fof30/fof/Input/Input.php in line: 37

Use of superglobals is strongly discouraged:

004 /pkg_gscrm_1.0.0/lib_fof30/fof/Input/Input.php in line: 40

Use of superglobals is strongly discouraged: 2 Errors -

001: /pkg_gscrm_1.0.0/lib_fof30/fof/Pimple/Container.php

GPL or compatible license was not found

002: /pkg_gscrm_1.0.0/lib_fof30/fof/Pimple/ServiceProviderInterface.php

GPL or compatible license was not found Pimple is released under the MIT license - did not change. 19 Errors -

001: /pkg_gscrm_1.0.0/lib_fof30/fof/Autoloader/Autoloader.php

The JEXEC security check was not found in this file. I did not change the code because of the docblock comment: // Do not put the JEXEC or die check on this file (necessary omission for testing) The other 18 files do not have the comment. Should I add? A note that "these are self contained files and do not need the JEXEC or die check" - I can do that, just tell me so.

nikosdion commented 6 years ago

I will NOT break FOF because a broken script written by JED outputs bullshit!!! Any developer who's not a complete idiot can see that all these are non-issues.

JRequest is there for old Joomla! versions. That's why it's inside an if-block. You can still use FOF 3 with Joomla! 3.3 on PHP 5.3 with PHP magic quotes enabled. Going through JRequest is the only way to not end up with broken input.

The InstallScript is written in a way which allows old Joomla! versions to gracefully print a message explaining the installation can't go through instead of dying with a white page. Remember that this method is only called on old versions of Joomla! (pre-3.4) affected by several bugs which prevent admin menu creation.

The Input package of course uses the superglobals since it, well, handles raw input by definition. It needs to go through superglobals when a script runs outside of Joomla!.

Pimple is, of course, released under MIT which is compatible with GPL.

The Autoloader does not have a JEXEC check because it's meant to be included by CLI and non-Joomla! scripts as well, including the automated tests.

Possibly other pure class or interface files do not have a _JEXEC check since it's not necessary. Pure. Class. Files. Non-executable. Do we even remember why the _JEXEC check is there or has Joomla! devolved into an Indian call centre where all everyone cares about is ticking boxes in some checklist they have no clue how it works?

@anibalsanchez please take a look at this. This is ridiculous. Rejecting this man's extension because the JED checker script's generalizations (which any decent developer would tell you are useless) misidentify FOF's code as broken is a bad joke! If the JED believes that FOF's code is wrong I want this IN WRITING AND WITH THE LEADERSHIP'S SIGNATURES so when I remove all that compatibility cruft you people complain about and break people's sites I can tell them to sue OSM for damages.

anibalsanchez commented 6 years ago

@go-source I think you misunderstood the cause of rejection. Please, send us a ticket to New Extension Support to clarify the issues.

FOF is certainly not related to your extension issues.

@nikosdion JED Checker applies several validations to identify bad practices, and it can generate false positives. So, we have to unzip every extension and check the code. In general, we can find code that was copy-pasted from any site of Internet. Of course, when we confirm that the files are from FOF (or from other libraries created to provide backward compatibility), we know that there's no need to check the code.

I appreciate your understanding.

anibalsanchez commented 6 years ago

One more note about licensing, all GPL-compatible libraries are ok. About MIT: https://www.gnu.org/licenses/license-list.en.html#Expat

go-source commented 6 years ago

thanks all. @anibalsanchez I do understand, no hard feelings here.