Michael Brohl <[email protected]> writes: > I was in fact asking for the discussion and review process to the > changes already introduced and committed in the mentioned Jira.
OK > It's a good approach to let fellow committers review such work before > it gets committed to the codebase. I agree in theory, but given the poor state of the codebase I feel that requiring that every single change is delayed for 2/3 days or more to let people review it is not manageable. The approach I am following in practice is to apply my own judgement to separate things that are safe and things that need to be discussed beforehand. I considered (maybe wrongly) that using <depends-on> in the “framework” and “applications” components directories was safe because it did not remove any feature. > This change will affect existing productive installations and should > not be made without proper documentation and clear instructions for > the user on how to migrate their installation to the new mechanism. > > To me it is not clear what we are gaining with this change. We are > removing one tag which is used and stable for years and introduce > another one which was not used before. Users are forced to migrate > their installations if they had custom modifications. I am confused about what you are talking about because I still don't see in what has already been committed in ‘trunk’ what is “affecting productive installations” because the “component-load.xml” feature is still here and users can still place their existing “component-load.xml” files and get the same behavior as before. > I did not look deeply into the changes yet but it looks strange that a > component like "product" or "order" only depends on "humanres". In > fact, they have more dependencies like "party". Given that > "depends-on" really means what it says and has a real difference to > the component-load.xml approach. The dependency relation is transitive meaning: if (x depends on y) and (y depends on z) then (x depends on z). It is possible to be explicit about the "transitive" dependencies by declaring them as direct dependencies when the fact that a direct dependency is depending on another one is an implementation details which might not hold in the future. For example: the Gradle plugin “groovy” depends on the Gradle plugin “java” but there is little chance that the “groovy” plugin will not depend on the “java” one in the future so it is safe to only declare a dependency on the “groovy” plugin. The current dependency declarations in framework/applications is a direct mapping of the order defined in the previous “component-load.xml” file which need to be refined because that order lacked information regarding the actual functional dependencies (which is the inherent downside of using a total order for partially ordered things). > The component-load.xml mechanism clearly shows the loading order of > the components which is an advantage over the cluttered information of > the depends-on mechanism. You will have to analyze every > ofbiz-component to see what's going on. You are correct that “component-load.xml” shows the precise loading order of components, however it lacks a precise definition of the actual functional dependencies between components. You can derive one valid loading order from the set of actual functional dependencies, however as stated above it is not possible to derive the functional dependencies from a “component-load.xml” file. Currently we lack a mechanism to display the global dependency graph which is important to for example to analyse why a component is loaded before or after another one. However this feature should be easy to implement and could be a requirement before removing the “component-load.xml” feature. > IMO these examples show that is in fact worth a discussion and should > not be a lone decision of a single committer. > > I would like to see this being reverted and proposed for discussion > and review before this is going to be introduced into the codebase. I have no problem reverting things, however to be legitim your call for revert should be backed by an actual proof that the “component-load.xml” feature has been removed (which is not the case) or that the component loading work has introduced a bug/regression. So I encourage you to try to backport one of your production “component-load.xml” on trunk and report a bug if necessary. Thanks. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
