On Dec 17, 2010, at 1:03 PM, Paul Berry wrote: > On Fri, Dec 17, 2010 at 12:00 PM, Luke Kanies <[email protected]> wrote: > Looks good, with one comment: > > I recommend that the apply_parameter method act on the created event, such as > by adding it to the 'events' array, or, preferably, having an 'add_event' > method that does this. > > I assume you are responding to the unfortunate code duplication between these > lines: > > events << apply_parameter(ensure_param, current_values[:ensure], > audited_params.include?(:ensure), historical_values[:ensure]) > synced_params << :ensure > > and these: > > events << apply_parameter(param, current_values[param.name], > audited_params.include?(param.name), historical_values[param.name]) > synced_params << param.name > > I considered a refactor along the lines you suggest, but I decided against it > for the following reason: if apply_parameter were responsible for adding the > event to the events array, then perform_changes would be relying for its > correctness on the knowledge that apply_parameter always adds exactly one > event to the event array, corresponding to the parameter in question. > Although there is a little bit of code duplication in the patch as submitted, > it seems like a small price to pay for the fact that the output of > apply_parameter is explicit, so you only need to look at the perform_changes > method to understand what events will be produced.
Actually, it's much less about the code duplication and more about the disconnect between event creation and what gets done with them. Again, it's almost identical to how the transactions used to work, and it caused a lot of pain in the end - the main point of this method is to perform action, not track events, so why not have a system on the side that handles the events transparently? -- The only really good place to buy lumber is at a store where the lumber has already been cut and attached together in the form of furniture, finished, and put inside boxes. --Dave Barry --------------------------------------------------------------------- Luke Kanies -|- http://puppetlabs.com -|- +1(615)594-8199 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
