search

balvi / cuba-component-declarative-controllers

CUBA component that allows to write generic features for a Controller and use them in a declarative way
5 stars 6 forks source link

upgrade to Cuba 7 #13

Open bresche opened 5 years ago

bresche commented 5 years ago
mariodavid commented 4 years ago

Hi @bresche,

I tried to use the latest snapshot version to switch the data-import plugin to CUBA 7.1. During that, I saw your commit (https://github.com/balvi/cuba-component-declarative-controllers/commit/17eda4df3d151f839b45fbac1c652ed1e24c5447) which contains breaking API changes to not support CUBA 6 based AbstractEditor and AbstractLookup anymore.

Is there any reason why you did that? It means that all the applications cannot use CUBA 6 based screens anymore if they want to use an annotation processing of this plugin.

Many of my application components currently use this app component, because in CUBA 6 there was no other way to achieve something similar. Here are a couple of examples:

This means that any app using one of my plugins is now forced to switch to CUBA 7 based UI controllers.

In the plugins I switched for CUBA 7 screens to provide an interface that will basically replace the need for this addon all together, since CUBA 7 APIs have equivalent (and type-safe) capabilities. However, for CUBA 6 based screens I did not want to break all the applications immediately.

But with the change you made, I cannot give that guarantee anymore.

Would it be possible to instead of introducing this breaking change just create a new variant for CUBA 7 next to the old one? Technically this should not be a problem.

Then the burden of changing the inheritance hierarchy happens for the app developer if they update to CUBA 7 screen API (but then they have to change a lot of stuff anyways) and not to the people that want to stay at the CUBA 6 based screens.

It would be great if you could re-think your decision in this regard.

Bye Mario

bresche commented 4 years ago

Hi Maro,

it would be better to ta have backward compability. So I create a copy of this repo:

https://github.com/balvi/cuba-component-declarative-controllers-cuba7

and reverted this repo to 0.8.0 release version. After upgrading the new repo to a better cuba 7 solution like screens mixins, the changes can merged ito this repo to have backward compability capabilites.

If you have some time you can look at the new repo and make some suggestions.

regards Matthias

mariodavid commented 4 years ago

Hi,

ok, sounds good. However, i'm not sure why you want to make a new repo for it?

I mean you can also just next to the existing structure create new super classes. E.g. instead of switching the super class of AnnotatableAbstractLookup from AbstractLookup to StandardLookup you could instead create a new class AnnotatableStandardLookup that will work against the new CUBA 7 API and leave the old code as it is.

Then it is also clear for the user that wants to upgrade their screens to a StandardLookup that the corresponding change should be taken for the screens with annotation usage as well.

But as I saw you already reverted the latest commit, so at least I can continue my work with the other plugins. Perhaps I will send you a PR for the 7.1. update of this plugin.

Thanks 👍

bresche commented 4 years ago

Hi,

I created a new repo to save my changes. We don't have any time to upgrade this plugin. Im glad that you do the migration.

regards Matthias

Am Di., 17. Sept. 2019 um 13:13 Uhr schrieb Mario David < notifications@github.com>:

Hi,

ok, sounds good. However, i'm not sure why you want to make a new repo for it?

I mean you can also just next to the existing structure create new super classes. E.g. instead of switching the super class of AnnotatableAbstractLookup from AbstractLookup to StandardLookup you could instead create a new class AnnotatableStandardLookup that will work against the new CUBA 7 API and leave the old code as it is.

Then it is also clear for the user that wants to upgrade their screens to a StandardLookup that the corresponding change should be taken for the screens with annotation usage as well.

But as I saw you already reverted the latest commit, so at least I can continue my work with the other plugins. Perhaps I will send you a PR for the 7.1. update of this plugin.

Thanks 👍

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/balvi/cuba-component-declarative-controllers/issues/13?email_source=notifications&email_token=AB5SQOLEI6NN5OCIL6KPX2DQKC3VXA5CNFSM4IG3XAW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD64FVLA#issuecomment-532175532, or mute the thread https://github.com/notifications/unsubscribe-auth/AB5SQOJGAV365FSB5LEJS4DQKC3VXANCNFSM4IG3XAWQ .