Re: [Perl-workflow-devel] [rt.cpan.org #38024] all conditions with 'test' attirbute appears to be cached as 'evaluate'

[Jonas B. N. <jo...@gm...>]

Hello All,

I have not received any feedback and I am reluctant to making a  
development release for a one-line patch, I was hoping we would be  
able to evaluate this and decide simply on the contents of the patch.

The issue is described below and the patch goes for line: 349 in HEAD

The line:
	name  => 'evaluate',

Roberts wants changed to:

	name => "_$condition_info->{test}",

I have not really gotten my head around the condition objects (I  
should do this soon since our docs needs a serious update).

But this comment just above makes me hesitate applying the patch:

	# Special case: a 'test' denotes our 'evaluate' condition

So I guess the evaluate name is supposed to be hard-coded. However  
Roberts patch does seem to offer some flexibility. But then the _  
(underscore) troubles me.

I am going to go over the tests covering this part later tonight, but  
if anybody has suggestions or opinions on this please feel free to  
write me (the list). Jim, I am particularly interested in your  
opinion :)

Now I will go for my run to clear my head then I will look at the code.


jonasbn

On 24/10/2008, at 23.55, Jonas Brømsø Nielsen wrote:

> Hiya Alll,
>
> We have received a patch from Robert Stockdale.
>
> If anybody has time to evaluate it, please feel free.
>
> jonasbn
>
> Begin forwarded message:
>
>> From: "Robert Stockdale via RT" <bug...@rt...>
>> Date: 29. sep 2008 21.48.42 GMT+02:00
>> To: undisclosed-recipients:;
>> Subject: [rt.cpan.org #38024] all conditions with 'test' attirbute  
>> appears to be cached as 'evaluate'
>> Reply-To: bug...@rt...
>>
>>      Queue: Workflow
>> Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=38024 >
>>
>> Hi Jonas,
>>
>> Sorry for the delay. I was able to confirm this bug.
>>
>> The name is being explicitly set to 'evaluate' in
>> Workflow::State::_create_condition_objects for all conditions that
>> specify 'test' as an attribute in the xml configuration. This,  
>> coupled
>> along with caching of condition results means that only the first
>> 'evaluate' condition will actually be evaluated and each subsequent  
>> call
>> will use the cached result.
>>
>> Attached is a patch that will resolve the issue. Essentially I am  
>> just
>> taking the value of the 'test' attribute and naming the condition  
>> that,
>> with one small exception. Perl seems to have some issue with keys  
>> that
>> start with '!' so I prefixed it with an underscore. I tested this  
>> change
>> and it is working in my dev environment.
>>
>> An alternative option would be to allow the user to specify the  
>> 'name'
>> attribute in the xml just like with any other condition. I did not
>> exercise this option.
>>
>> -Bob
> <evaluate_condition_name.patch>