Update logger prefix if uid is set inside rule block #252
Conversation
Should we update it immediately when |
jimtng
force-pushed
the
rule-uid-logging
branch
2 times, most recently
from
7472bb5 to
5042325
Compare
That's a nice challenge :) At first I thought I had to define #uid with a plain method, but then I thought of another way. |
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
jimtng
force-pushed
the
rule-uid-logging
branch
from
5042325 to
47b2014
Compare
| @@ -30,6 +32,7 @@ def prop(name) | |||
| elsif block | |||
| instance_variable_set(:"@#{name}", block) | |||
There was a problem hiding this comment.
@ccutrer, This is unrelated to the PR, but I just noticed this. Shouldn't we call the block here instead?
| instance_variable_set(:"@#{name}", block) | |
| instance_variable_set(:"@#{name}", block.call(name)) |
There was a problem hiding this comment.
I'm guessing that the idea was to pass a block to the property, e.g.
rule do
tags { # build and return the required tags }
end
Since this isn't documented I doubt anyone uses it this way.
There was a problem hiding this comment.
it would make sense to assign the block itself to the ivar, if you then actually call that block when reading the property, and if there was actually some advantage to deferring execution of that block until the property is read, but that's not really the case. execution could be deferred, but only until the end of the rule block, which doesn't seem useful. and like you said, not document. so I vote we just remove this functionality completely
| @@ -30,6 +32,7 @@ def prop(name) | |||
| elsif block | |||
| instance_variable_set(:"@#{name}", block) | |||
There was a problem hiding this comment.
it would make sense to assign the block itself to the ivar, if you then actually call that block when reading the property, and if there was actually some advantage to deferring execution of that block until the property is read, but that's not really the case. execution could be deferred, but only until the end of the rule block, which doesn't seem useful. and like you said, not document. so I vote we just remove this functionality completely
No description provided.