I am still not sure why we need the new mechanism.

And if we decide to use both, we should make sure that the old version is the default at least for the lifecycle of one release with proper an clear dopcumentation for users.

Thanks,

Michael


PS: I'm asking myself why some people have such a big problem reverting their work if others object against it. If there was no review/discussion/consensus for a new feature, it should simply not go into the codebase and should at least be reverted if there are objections.

It's tiring to discuss this afterwards and if the people objecting are not persistent enough, the code simply stays.

If you would build a house together with others, noone would ever have the idea to change things overnight without discussing it first.


Am 13.12.19 um 16:32 schrieb Jacques Le Roux:
Hi All,

Have we a lazy consensus here?

Jacques

Le 10/12/2019 à 06:17, Jacques Le Roux a écrit :
Hi Michael, Mathieu,

Why not keeping both features and have a simple way (if it does not exist yet) to choose the one to use?

BTW Michael,  you are certainly aware, you have a mail certificate issue.

Thanks

Jacques

Le 09/12/2019 à 08:08, Michael Brohl a écrit :
Hi Mathieu,

inline...

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.

This is how community projects work!

If you want to introduce new features or important changes to the codebase, you will have to propose them and stand the discussion (and maybe delays of a few weeks if not months). It is the responsibility of the contributor to prove his solution being a good fit for the project beforehand.

The quality of code you mentioned partly is a result of times when a few committers simply forced their changes into the codebase whether the community liked it or not.


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.

As committers we should act as positive examples on how good community work should be lived. New features, especially if they have an impact on the users, should always be discussed beforehand. And if they get accepted, it should also include a good documentation and migration guide for the users.



I considered (maybe wrongly) that using <depends-on> in the “framework”
and “applications” components directories was safe because it did not
remove any feature.

It *does* remove a feature from a users perspective. The component-load.xml will be gone and custom enhancements will not work ootb after the change.


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.

See above. Users are driving serious business with the OFBiz platform and we should have this in mind when we are doing changes to the codebase.

It's not just a technical change, it affects projects, needs migration testing etc. and therefore it should be discussed thoroughly if this is a good and needed change.


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 current implementation of "depends-on" does also lack information of functional dependencies because they are not clearly visible at teh place where they are defined.

IMO it does not improve the transparency of the dependencies better than the "component-load" approach.



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.

See my statements about community work above.


So I encourage you to try to backport one of your production
“component-load.xml” on trunk and report a bug if necessary.

Did you test how "depends-on" is working together with the "component-load" mechanism?

It is not my responsibility to prove that it is not working, it is yours to prove that it works.



Thanks.

Let me say that I really appreciate your work for the project, Mathieu.

I'm simply trying to keep up good community work and feel responsible for the users of the project, hence my objections.

Thanks,

Michael







Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

Reply via email to