hi jakob, i suggest the same approach like before. last time leo requested some changes and had to start the vote (with a short description) and this time it's your turn. if you feel that we need a vote about it, please feel free to start one.
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/11 Jakob Korherr <[email protected]> > Hi, > > Leo, I really get your point that we can't change this stuff. Changing > SPI stuff (even just renaming packages) will break application server > integation code, we all got it now.. > > That's why I suggested (a few mails ago, but unfortunately too vague) > keeping the package names *.spi, *.spi.impl and *.config.element as > is, but still moving *.spi and *.config.element to a new module called > myfaces-spi. This alone will let us be able to remove shaded-impl, no > code change is actually required, just moving some classes from > myfaces-impl into myfaces-spi. In the end (--> the myfaces-impl jar), > it will all be back in myfaces-impl again, because of shade. > > I will provide a new patch by tomorrow and then start a vote for it. > There really is no technical reason for not committing such a solution > at this point. > > Regards, > Jakob > > 2011/7/11 Leonardo Uribe <[email protected]>: > > Hi > > > > I'll be clear and direct. What makes statements true or false are the > > reasons behind it. Until this moment, you are not question the reasons > > behind the reasoning proposed. > > > > To be short, my argumentation is we can't change for now: > > > > 1. Everything inside org.apache.myfaces.spi > > 2. LifecycleProvided > > 3. Everything inside org.apache.myfaces.config.element > > > > because JSF 2.1 is a maintenance release (not a mayor release) which > > already has it first version (but even without a version). Do that > > will create bugs on server integration code. The problem is there is > > no way to detect such changes or create a proper patch from the server > > side point of view without use some ugly reflection code and that > > really s...cks!. > > > > Let it to JSF 2.2 (which is another JSR) sounds better because in that > > time I think we can get rid of implee6 too in one move!. In that > > version, just change the poms, and move all code to impl and that's > > it. > > > > In conclusion, here we have a technical restriction that doesn't allow > > us to move further, so we can't really make a vote because there is no > > decision to make, we just can't change the code!. The idea of an API > > in impl module is precisely make the "promise" that we will be nice > > and do not change that stuff until the next major version. > > > > Unfortunately there is nothing we can do in 2.1.x branch. > > > > regards, > > > > Leonardo Uribe > > > > 2011/7/11 Jakob Korherr <[email protected]>: > >> 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 > >> > > > > > > -- > Jakob Korherr > > blog: http://www.jakobk.com > twitter: http://twitter.com/jakobkorherr > work: http://www.irian.at >
