On 25/07/2011, at 7:51 AM, Adam Murdoch wrote:

> The whenObjectAdded()/whenObjectRemoved() methods are really intended for 
> notification after the fact, not validation before the fact.
> 
> I'd be tempted to add beforeObjectAdded()/beforeObjectRemoved() notifications 
> which can be used for validation, perhaps not on the public interface, but 
> just as methods on DefaultDomainObjectCollection.

Perhaps we should add general protected hooks for this on 
DefaultDomainObjectCollection, and use an inner subclass of this in 
Configuration that overrides these methods. Which now I think of it, is 
probably what you are saying.

> Just btw, even if we fix this particular problem, the validation on 
> configuration falls a bit short:
> * We don't check for changes to the dependency objects. For example, I can 
> point a project dependency at a different configuration, or change the 
> exclusions for an external dependency.
> * We don't check for changes to the target configuration of project 
> dependencies. Or their transitive project dependencies.
> * We don't check for changes to inherited configurations.
> 
> I wonder if we should instead use a caching approach, where we allow any 
> changes to configuration, but cache and reuse the resolved dependency if the 
> unresolved graph of dependencies has not changed. There are 2 benefits to 
> this approach:
> * We have more options to reuse the cached result. For example, if my project 
> does not declare any runtime dependencies, I can reuse the result from 
> resolving the compile configuration. We can also reuse the result across 
> builds, if we're running in the daemon.
> * We don't have to try to validate all the possible way that the graph can be 
> constructed. We just need to use the result. This gives us some flexibility 
> to add some more interesting ways to construct the graph (in particular, 
> third party extensions).
> 
> Having said this, I don't think we should try to fix this now, we should just 
> replicate the existing behaviour.

I'm tempted to leave the add/remove methods on configuration for m4 and leave 
that as the documented way as I've run out of time to work on this area right 
now.

-- 
Luke Daley
Principal Engineer, Gradleware 
http://gradleware.com

Reply via email to