Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [papyrus-rt-dev] Request for Commit Process Exception: Façade Refactoring

Hi Christian,

I could only have a quick look to your gerrit (even if this is only mainly generated code / icons / tests ;-) ). At the first glance, I say +1 to commit (I will also physically do it on the gerrit).
There are JUnit tests, and I trust you to have them green before posting this message.

I find the solution very elegant. I love for example the databinding for the UMLRTElementProperties in tooling. The documentation that comes around is also very detailed. There may be some impacts, or at least, some places in the code that will benefit from this update, so I would definitely push for it.

The only glitch I could see when browsing the code from a very distant view is the loss of overlays in item providers because the overlayIcon is overriden without call to super(). For example, I don't think that a Capsule will get a submodel decoration, but that would be only a tiny glitch compared to gains.

Finally, thanks for posting this message before pushing to master directly. No worry for getting over gerrit, this tool is supposed to help, not to block ;) 

Cheers,
Rémi 

  

2017-01-31 16:08 GMT+01:00 Christian Damus <give.a.damus@xxxxxxxxx>:
Hi, Team,

In order not to block the fixing of bugs in capsule structure inheritance (e.g., 511380) and to get past the hurdle of the modelled façade refactoring, I would like to merge Gerrit 89958 as soon as possible.  After, of course, review by some suitable volunteer (don't worry — at least half of it comprises icon files, generated model interfaces, the generated UML extensions that don't have custom code, plus loads of tests 😉).

The problem is that this commit cannot pass the Gerrit builds as currently structured and I don't see any way to tease it apart into a sequence of commits that can each be individually reviewed and merged before the next.  The reason is that, although from an API perspective for most clients the modelled façade is a drop-in replacement for the API currently on master, because now the façade objects are EObjects, they are recognized differently by UI frameworks such as the Papyrus Properties View.  Trying to commit the façade API first would break the UI tests, but the UI cannot be changed in advance because the façade objects would not yet be EObjects.  Moreover, because the modelled façade is a drop-in replacement for the existing API, I cannot first add the new façade in its entirety, then switch the UI over to using it, and finally delete the old façade because the API names are all identical:  they cannot both be built and deployed together (that is the whole point of the drop-in replacement).

The only way I see out of this is just to push the change directly to master once the reviewer(s) is/are satisfied with it.  All of our JUnit tests for affected components pass when run locally and the tooling seems still to work.  Unless somebody can suggest a solution that I'm not seeing?

Comments welcome, as are reviewers, of course.

Thanks,

Christian

_______________________________________________
papyrus-rt-dev mailing list
papyrus-rt-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/papyrus-rt-dev




--
Remi Schnekenburger

Senior Software Architect / General Manager
EclipseSource Paris

Email: rschnekenburger@xxxxxxxxxxxxxxxxx
Web: http://eclipsesource.com/paris
Mobil: +49 89 21 555 30 - 25 
Phone: +49 89 21 555 30 - 25
Fax: +49 89 21 555 30 - 19

EclipseSource France SAS
7 rue de la Croix Martre
91873 Palaiseau

General Manager: Remi Schnekenburger

Back to the top