myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jakob Korherr <jakob.korh...@gmail.com>
Subject Re: Proposal: Get rid of shaded-impl module in core 2.1.x
Date Mon, 11 Jul 2011 09:51:08 GMT
Hi,

> 1. All classes in org.apache.myfaces.spi.

I did not change anything here, just internal imports (from *.spi.impl
to *.spi.util)!

> 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.

> 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).

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. I have to say that I won't support a
2.1.2 release including the shaded-impl module.

Regards,
Jakob

2011/7/10 Leonardo Uribe <lu4242@gmail.com>:
> 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 <gerhard.petracek@gmail.com>:
>> 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 <lu4242@gmail.com>
>>>
>>> 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 <werner.punz@gmail.com>:
>>> > 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 <lu4242@gmail.com <mailto:lu4242@gmail.com>>
>>> >>
>>> >>    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 <gerhard.petracek@gmail.com
>>> >>    <mailto:gerhard.petracek@gmail.com>>:
>>> >>     > +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 <jakob.korherr@gmail.com
>>> >>    <mailto:jakob.korherr@gmail.com>>
>>> >>     >>
>>> >>     >> 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

Mime
View raw message