Hi, No, sorry Leo, they are not enough. Frankly, I don't understand why you are blocking this solution. It is easy, it does not break anything (if we do not change the package names), it is a lot more clean and we can get rid of the shaded-impl module. If we don't do this now, we will maybe have to use this ugly module for a long time..
And yes: in my opinion it is an epic fail. If you think otherwise, that's ok, but just because you say so and I don't does not make your statement true. I think we have to start a vote for this one just like we did with the resource-handler stuff. Regards, Jakob 2011/7/11 Leonardo Uribe <[email protected]>: > Hi > > 2011/7/11 Jakob Korherr <[email protected]>: >> Hi, >> >>> 1. All classes in org.apache.myfaces.spi. >> >> I did not change anything here, just internal imports (from *.spi.impl >> to *.spi.util)! > > It is questionable to create .spi.util. After all, it is not supposed > that package be consumed by container integration code, so it should > be on spi.impl. > >> >>> 2. All classes that has to be with LifecycleProvider (@PostConstruct >>> annotation). Such classes should be on spi package, but note spi work >>> started after 2.0 release, so this should wait to a major version. >> >> Geronimo (for which we did the SPI stuff) includes MyFaces 2.0.x. Here >> we are talking about 2.1.x. Furthermore, one call to organize imports >> does the trick, so I do not see a problem here. > > Look the page of JSR-314 spec http://www.jcp.org/en/jsr/summary?id=314 > > You will notice 2.1.x is not a new JSR like the future JSR-344 (JSF > 2.2), and instead is tagged as "Maintenance Release 2". So, to be > consistent, it should be possible to change between 2.0 and 2.1 on the > same server. That's the most important reason to be so conservative or > strict with 2.1. > >> >>> 3. All classes on org.apache.myfaces.config.element. These classes are >>> an interface to manipulate faces-config.xml files with java code, and >>> spi interfaces provide the hooks to get them, so in theory we can add >>> methods there, but relocate this package to >>> org.apache.myfaces.spi.data is not necessary. I think the package name >>> is correct. >> >> OK, fine. I thought the relocation to org.apache.myfaces.spi.* would >> fit better, since it is the myfaces-spi module and, again, since we're >> talking about 2.1.x, not 2.0.x here. But if you want to keep the >> package-name, I have no problem with that. >> org.apache.myfaces.config.element is fine too, however, you may not >> expect it to be in the myfaces-spi module. >> >>> [...] Considering this, I think the costs >>> involved on the changes proposed are too big compared with the >>> benefits, which are very limited and almost inexistent from user point >>> of view. >> >> From a user point of view: maybe yes. But from a developer point of >> view myfaces-shaded-impl is an epic fail. I know it was an easy an >> fast solution at the time we got 2.1.0 out, but for the long term this >> has to be changed. Please think about it, you use 2.0.5 (or any other >> _previous_ version) in myfaces-impl-ee6 as if it was 2.1.x-SNAPSHOT. >> Thus you use internals of previous versions which might not even be >> there anymore in the current 2.1.x-SNAPSHOT. And the worst part of it: >> you don't even see it at build time, b/c it's a separate module and >> myfaces-impl-ee6 is shaded into myfaces-impl (and shade does not warn >> about this kind of stuff, of course). >> > > I know the hack done to compile 2.1 is not very clean, but it works. > The idea is replace 2.0.5 ref with 2.1.0 in future versions. Note > myfaces-shaded-impl is a module that is just for allow compile > myfaces-impl-ee6, and nobody else. It is not "an epic fail", because > it does not cause any changes on the code that cause problems. > >> Considering this, it was ok to create shaded-impl in order to get the >> 2.1.0 release out, but for future releases (maybe also towards 2.2.0), >> this must be done in another way. > > In fact, the idea is do something in 2.2.x, but that will take some > time, so maybe we can keep in mind those changes until get there. > >> I have to say that I won't support a >> 2.1.2 release including the shaded-impl module. > > I hope my arguments could be enough to convince you about the opposite. > > regards , > > Leonardo Uribe > >> >> Regards, >> Jakob >> >> 2011/7/10 Leonardo Uribe <[email protected]>: >>> Hi Gerhard >>> >>> Ok, in theory the parts that we should not change, or otherwise that >>> will trigger a change on JEE containers are: >>> >>> 1. All classes in org.apache.myfaces.spi. >>> 2. All classes that has to be with LifecycleProvider (@PostConstruct >>> annotation). Such classes should be on spi package, but note spi work >>> started after 2.0 release, so this should wait to a major version. >>> 3. All classes on org.apache.myfaces.config.element. These classes are >>> an interface to manipulate faces-config.xml files with java code, and >>> spi interfaces provide the hooks to get them, so in theory we can add >>> methods there, but relocate this package to >>> org.apache.myfaces.spi.data is not necessary. I think the package name >>> is correct. >>> >>> regards, >>> >>> Leonardo Uribe >>> >>> 2011/7/9 Gerhard Petracek <[email protected]>: >>>> hi leo, >>>> thx for checking it. >>>> it would be great, if you can post a list of parts (not classes) which >>>> would >>>> break. so we can discuss this topic in a more concrete manner. >>>> regards, >>>> gerhard >>>> http://www.irian.at >>>> >>>> Your JSF powerhouse - >>>> JSF Consulting, Development and >>>> Courses in English and German >>>> >>>> Professional Support for Apache MyFaces >>>> >>>> >>>> 2011/7/10 Leonardo Uribe <[email protected]> >>>>> >>>>> Hi >>>>> >>>>> Good to know that. Unfortunately, do a change on the spi related >>>>> classes will break existing integration code with containers like >>>>> Geronimo, JBoss and others too. Considering this, I think the costs >>>>> involved on the changes proposed are too big compared with the >>>>> benefits, which are very limited and almost inexistent from user point >>>>> of view. >>>>> >>>>> regards, >>>>> >>>>> Leonardo Uribe >>>>> >>>>> 2011/7/8 Werner Punz <[email protected]>: >>>>> > I have no problem if you break Ext-Script, after all I program against >>>>> > the >>>>> > impl, so whatever change you do I have to patch it in my codebase >>>>> > anyway. >>>>> > Don´t feel stopped by it. >>>>> > >>>>> > Werner >>>>> > >>>>> > >>>>> > Am 08.07.11 18:27, schrieb Gerhard Petracek: >>>>> >> >>>>> >> hi, >>>>> >> >>>>> >> thx for reviewing the changes. >>>>> >> for sure we can discuss when we are going to do such changes. however, >>>>> >> if ext-script is the only reason against it, we can do it right now >>>>> >> imo. >>>>> >> there is no problem with shipping a small update of ext-script and the >>>>> >> user base is currently small enough to communicate this change easily. >>>>> >> >>>>> >> regards, >>>>> >> gerhard >>>>> >> >>>>> >> http://www.irian.at >>>>> >> >>>>> >> Your JSF powerhouse - >>>>> >> JSF Consulting, Development and >>>>> >> Courses in English and German >>>>> >> >>>>> >> Professional Support for Apache MyFaces >>>>> >> >>>>> >> >>>>> >> >>>>> >> 2011/7/8 Leonardo Uribe <[email protected] <mailto:[email protected]>> >>>>> >> >>>>> >> Hi >>>>> >> >>>>> >> In principle I agree with this change, but after a quick patch >>>>> >> review >>>>> >> I saw some package relocation for some classes (from >>>>> >> org.apache.myfaces.config.element to org.apache.myfaces.spi.data). >>>>> >> Those changes will break extensions scripting code. Such changes >>>>> >> should be done between major versions (2.2.0). Please do not change >>>>> >> the package names. >>>>> >> >>>>> >> regards, >>>>> >> >>>>> >> Leonardo Uribe >>>>> >> >>>>> >> 2011/7/8 Gerhard Petracek <[email protected] >>>>> >> <mailto:[email protected]>>: >>>>> >> > +1 >>>>> >> > regards, >>>>> >> > gerhard >>>>> >> > >>>>> >> > http://www.irian.at >>>>> >> > >>>>> >> > Your JSF powerhouse - >>>>> >> > JSF Consulting, Development and >>>>> >> > Courses in English and German >>>>> >> > >>>>> >> > Professional Support for Apache MyFaces >>>>> >> > >>>>> >> > >>>>> >> > >>>>> >> > 2011/7/8 Jakob Korherr <[email protected] >>>>> >> <mailto:[email protected]>> >>>>> >> >> >>>>> >> >> Hi guys, >>>>> >> >> >>>>> >> >> We currently use the shaded-impl module in core 2.1.x to solve a >>>>> >> >> cyclic dependency problem. However, this introduces redundancy >>>>> >> and >>>>> >> >> complexity and may lead to some strange issues. This can be >>>>> >> solved by >>>>> >> >> the introduction of a myfaces-spi module (which is then packed >>>>> >> >> together with myfaces-impl, just like myfaces-impl-ee6 is). >>>>> >> >> >>>>> >> >> Please see MYFACES-3211 [1] for a detailed description of the >>>>> >> solution >>>>> >> >> and a patch for it. >>>>> >> >> >>>>> >> >> If there are no objections, I will commit this patch soon! >>>>> >> >> >>>>> >> >> Regards, >>>>> >> >> Jakob >>>>> >> >> >>>>> >> >> [1] https://issues.apache.org/jira/browse/MYFACES-3211 >>>>> >> >> >>>>> >> >> -- >>>>> >> >> Jakob Korherr >>>>> >> >> >>>>> >> >> blog: http://www.jakobk.com >>>>> >> >> twitter: http://twitter.com/jakobkorherr >>>>> >> >> work: http://www.irian.at >>>>> >> > >>>>> >> > >>>>> >> >>>>> >> >>>>> > >>>>> > >>>>> > >>>> >>>> >>> >> >> >> >> -- >> Jakob Korherr >> >> blog: http://www.jakobk.com >> twitter: http://twitter.com/jakobkorherr >> work: http://www.irian.at >> > -- Jakob Korherr blog: http://www.jakobk.com twitter: http://twitter.com/jakobkorherr work: http://www.irian.at
