User tests: Successful: Unsuccessful:
This patch removes some unused (javascript/PHP) code in the following views
Full b/c, no problems are expected
none
I'm resending failed payloads right now. See joomla/jissues#617 for more.
I am confused by this PR - if the code is not used then why was it
refactored and how was it actually tested with PR #5041, #5255 and quote a
few other PR
On 6 February 2015 at 13:56, Michael Babker notifications@github.com
wrote:
I'm resending failed payloads right now. See joomla/jissues#617
joomla/jissues#617 for more.—
Reply to this email directly or view it on GitHub
#5994 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
@brianteeman The refactor of all these views was mainly focused on two parts:
introduce formvalidator
use the joomla API for the inline scripts.
The later (which is the case here) was a one-to-one transformation from inline scripts to addScriptDeclaration(), nothing more nothing less
My concern is that the code was "tested" - how could it be if the code isnt
used - Its a general comment about our testing procedures
On 6 February 2015 at 14:11, Dimitris Grammatiko notifications@github.com
wrote:
@brianteeman https://github.com/brianteeman The refactor of all these
views was mainly focused on two parts:
introduce formvalidator
use the joomla API for the inline scripts.
The later (which is the case here) was a one-to-one transformation from
inline scripts to addScriptDeclaration(), nothing more nothing less—
Reply to this email directly or view it on GitHub
#5994 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Tested => worked without error
Reviewed => Code was inspected
The problem wasn’t the testers in this particular event. Also no blame on the commiter or anyone that reviewed that code, because just by reading the changes made you couldn’t find any flaw because the initial code was useless.
My 2c
This is the problem "in general" that concerns me.
People are regularly applying patches and saying it works without
confirming beforehand that they have observed the issue and have observed
the changed behaviour. Joomla lives in a web browser - just reading the
code is not enough
On 6 February 2015 at 14:23, Dimitris Grammatiko notifications@github.com
wrote:
Tested => worked without error
Reviewed => Code was inspected
The problem wasn’t the testers in this particular event. Also no blame on
the commiter or anyone that reviewed that code, because just by reading the
changes made you couldn’t find any flaw because the initial code was
useless.
My 2c—
Reply to this email directly or view it on GitHub
#5994 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
The problem is that after introducing the search tools the old code wasn't removed. The fact that we don't use the search tools everywhere and in the other situations (not used search tools) these code is needed doesn't makes it easier. Without really looking deep into the code it is hard to spot that this is unused.
Category | ⇒ | Administration Code style |
Status | New | ⇒ | Pending |
@test I cant see any changes in ordering so I guess this is a successful test
As we have two tests I am setting this RTC
Status | Pending | ⇒ | Ready to Commit |
Labels |
Added:
?
?
|
Labels |
Added:
?
?
|
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-02-12 08:20:13 |
Tested and works for me but don't know why I didn't find this issue on http://issues.joomla.org/tracker/joomla-cms/