Community
Participate
Working Groups
commit 902b1efd5d2a272a8216c1d38bb0ba1f6190c6b7 Author: Jan Koehnlein <jan.koehnlein@itemis.de> 2011-07-05 13:12:44 Committer: Jan Koehnlein <jan.koehnlein@itemis.de> 2011-07-05 13:12:44 Parent: 5e7930ebb80cd46601c7de7abcac62e8e5101ed1 ([refactoring] Proxy resolution for references done right) Child: 01ef86dcdadd1829b6ab30e3b1ac716eb64a149d ([xtext][refactoring] Maintain backwards compatibility with generated code) Branches: origin/master, origin/releng, master [refactoring] Refactored strategies changes origin/releng, which appears to be your maintenance branch without any reference to any Bugzilla or review for an API change. Surely internal APIs can be changed in 2.1 but not in 2.0.*? The change breaks existing code since a) an incompatible constructor signature is made b) access to a field is changed Consequently MDT/OCL maintenance builds fail, and it will be impossible to install MDT/OCL 3.1.0 editors on Xtext 2.0.1.
Ed, we don't have a maintenance branch so all the changes for 2.0.1 appeared in master. What we will tag as 2.0.1 is commit http://git.eclipse.org/c/tmf/org.eclipse.xtext.git/commit/?id=b709b4e52a17cd989a3b7e423d8cd678a6ec5395 . Could you please double check that the OCL rename support is broken with that commit, too?
(In reply to comment #0) > Consequently MDT/OCL maintenance builds fail, and it will be impossible to > install MDT/OCL 3.1.0 editors on Xtext 2.0.1. This seems much too strong. I was assuming that P2 would blow up, but P2 won't check class signatures so the install should succeed. It is only when a user attempts a refactor that a bad class exception is likely to be thrown. (In reply to comment #1) > Ed, we don't have a maintenance branch so all the changes for 2.0.1 appeared in > master. I'm not sure how you can manage without a maintenance branch, unless you are going to try to maintain all internal APIs stable throughout 2.1, which will be really hard if you're continuing to make good improvements... but that's your releng policy. What we will tag as 2.0.1 is commit > http://git.eclipse.org/c/tmf/org.eclipse.xtext.git/commit/?id=b709b4e52a17cd989a3b7e423d8cd678a6ec5395 > . Could you please double check that the OCL rename support is broken with that > commit, too? By visual inspection, reinstatement of the constructor resolves one issue, but there were no getters in 2.0 so direct access to the protected fields was mandatory. These are now private, so the code should still fail. Have you tried the API tooling? I suspect that it would report this change. [The OCL rename support is certainly broken, because it was never not broken. It was clear that rename support was too internal to justify trying to debug this with many other things to be done as well before Indigo. The problem is that the trial code that sometimes renamed successfully but with poor ergonomics is still present on one of the less used editors.]
(In reply to comment #2) > Have you tried the API tooling? I suspect that it would report this change. API tooling does not complain about changes in internal packages. If they were considered to be API it would be pointless to mark them as internal. I'll close this one as works for me (because I tried could successfully use Xtext 2.0.1 with a language that was developed against 2.0.0 - including rename support). Please reopen if you face any problems in practice when you try to build / run against the 2.0.1 release.
In practice we had the build problem. > Error: file<https://hudson.eclipse.org/hudson/job/buckminster-mdt-ocl-3.1-maintenance/ws/org.eclipse.ocl.git/examples/org.eclipse.ocl.examples.xtext.oclstdlib.ui/src/org/eclipse/ocl/examples/xtext/oclstdlib/ui/refactoring/OCLstdlibRenameStrategy.java,> line 70: The field AbstractRenameStrategy.targetElementOriginalURI is not visible > Error: file<https://hudson.eclipse.org/hudson/job/buckminster-mdt-ocl-3.1-maintenance/ws/org.eclipse.ocl.git/examples/org.eclipse.ocl.examples.xtext.oclstdlib.ui/src/org/eclipse/ocl/examples/xtext/oclstdlib/ui/refactoring/OCLstdlibRenameStrategy.java,> line 71: The field AbstractRenameStrategy.targetElementOriginalURI is not visible > Error: file<https://hudson.eclipse.org/hudson/job/buckminster-mdt-ocl-3.1-maintenance/ws/org.eclipse.ocl.git/examples/org.eclipse.ocl.examples.xtext.oclstdlib.ui/src/org/eclipse/ocl/examples/xtext/oclstdlib/ui/refactoring/OCLstdlibRenameStrategy.java,> line 94: The method getNameAttribute(EObject) of type OCLstdlibRenameStrategy must override a superclass method > Error: file<https://hudson.eclipse.org/hudson/job/buckminster-mdt-ocl-3.1-maintenance/ws/org.eclipse.ocl.git/examples/org.eclipse.ocl.examples.xtext.oclstdlib.ui/src/org/eclipse/ocl/examples/xtext/oclstdlib/ui/refactoring/OCLstdlibRenameStrategy.java,> line 98: The method getNameAttribute() in the type AbstractRenameStrategy is not applicable for the arguments (EObject) The retraction of the rename support in Bug 354083 means we do not have build problem today, but it requires us to make a very obscure change in an API that is not really appropriate for a service release. For a similar problem in our own SR code, we add default (wrong) visibility methods in the SR branch to avoid API 'breakage', while making them protected in the release branch. Making protected internal fields private with getters is certainly good for 2.1, but for 2.0.x I feel that you should make the absolute minimum changes necessary to fix bugs/offer simple extra facilities. Changing code just for encapsulation integrity does not seem appropriate.
2.0.1 is already out. Not worth the hassle for pedantic change unless someoen really needs it.
Closing all bugs that were set to RESOLVED before Neon.0
