On 7/9/2010 8:32 AM, Adam Heath wrote:
Adrian Crum wrote:
On 7/9/2010 7:00 AM, Adam Heath wrote:
BJ Freeman wrote:
I would like to see one place that enables them all
but if that is not enabled then web.xml would.
with as many components(over 30) I have I would like the all function.
I haven't looked at at any of the code, but what you guys want here is
boolean logic with tri-states.
if web.xml doesn't have the setting, use the global value.
if there is no global value, then default to false.
So you would remove all these settings from all components, then they
would all use the value of the global setting.
But if a pariticular web.xml has the value set, then it would become
disconnected from the global setting.
That was the behavior before Hans changed it. Now local settings are
ignored if the global setting is true.
And it isn't tri-state - it's more like inheritance. The
widget.properties setting is the default. You can override that setting
in web.xml. On top of that you can override both of those settings with
a parameter or context variable.
If that's the case, then the change should be reverted.
In such a tri-state system, either the global setting is a higher
priority, or the local setting is. There is no way the global could
be higher in one situation, and the local higher in another.
Before this change, the local setting had priority. After this
change(based on discussion I've seen here), a global setting has priority.
That was the problem Hans encountered. His custom application's web.xml
file overrode the default setting, but he wasn't aware that the setting
existed.
There is no particular reason to have one setting have more priority
than the other, so in those situations, the status quo should be
followed, which means don't change it. So, the patch should be reverted.
Now, a could different separate discussions could be started, if: one,
this global/local setting doesn't follow the pattern in other parts of
ofbiz, or two, the system should be changed to allow global settings
to take priority.
That discussion has occurred in the past and it certainly could be
discussed again. The original code had the HTML comments default to off
globally, and somewhere along the line the setting was changed to on.
The Example component always had the comments disabled - so its behavior
didn't differ from the other components until the global setting was
changed.
The Example component could be changed to have the HTML comments
enabled, and maybe have them disabled in one screen with an explanatory
comment that it is set that way to demonstrate the feature.
ps: I'm just trying to restate what I have seen said in this thread,
in a way that everyone involved can understand and agree on. I have
tried not to take any particular side(that sounds combative), or make
any kind of decision.
It's unfortunate that anyone in the community should feel the need to
avoid taking sides. From my perspective, there are no "sides" in this
discussion. I am simply trying to educate a fellow committer on the
design of a piece of code I authored. In addition, I'm also trying to
reinforce the concept of "understand the code before you change it."
-Adrian
