I think you are both right. I can understand that copying code is really ugly, 
but otoh Leos argument is also pretty strong.

There is a solution for this. Cut off the shared parts and move it into an own 
module. 

This sounds easy but isn't always doable. But it might be worth a try.

LieGrue,
strub


>
>From: Jakob Korherr <[email protected]>
>To: MyFaces Development <[email protected]>
>Sent: Mon, July 26, 2010 10:32:31 PM
>Subject: Re: Use maven-shade-plugin to prevent duplicate code
>
>Why would you like to have any duplicate code? This should not be anyone's 
>target in my opinion...
>
>
>
>2010/7/26 Leonardo Uribe <[email protected]>
>
>Hi
>>
>>Yes, it is true, that myfaces-test is used for testing myfaces core, but in 
>>theory, myfaces-test should be used to test jsf stuff without rely on a 
>>specific 
>>
>>jsf implementation.
>>
>>In this case I think (and it is my personal opinion) it is better to have 
>>some 

>>duplicate code and keep things simple. Use maven-shade-plugin to deal with 
>>shared code is another different history.
>>
>>regards,
>>
>>Leonardo Uribe
>>
>>
>>2010/7/26 Jakob Korherr <[email protected]>
>>
>>
>>Actually this already is the case: MyFaces-test is used for testing MyFaces 
>>core.
>>>
>>>
>>>Regards,
>>>Jakob
>>>
>>>
>>>2010/7/26 Rudy De Busscher <[email protected]>
>>>
>>>Hi Jakob,
>>>>
>>>>So it was never the idea that MyFaces Test (and maybe the GSOC testing 
>>>>effort) 
>>
>>>>will be used to supply the test infrastructure for MyFaces Core?
>>>>
>>>>In that case : MyFaces Core can supply code.
>>>>
>>>>Regards
>>>>Rudy.
>>>>
>>>>
>>>>
>>>>On 26 July 2010 22:01, Jakob Korherr <[email protected]> wrote:
>>>>
>>>>Hi Rudy,
>>>>>
>>>>>Yes we totally have to be careful with circular dependencies, but it 
>>>>>should not 
>>>>
>>>>>be that big problem.
>>>>>
>>>>>Actually I think that the opposite is true. MyFaces core is the JSF 
>>>>>implementation and MyFaces test provides the Mock classes for JSF, and for 
>>>>>implementing these Mock classes it can reuse some functionality already 
>>>>>present 
>>>>
>>>>>in MyFaces core (e.g. the real classes which should be mocked). IMO this 
>>>>>is the 
>>>>
>>>>>way it should be.
>>>>>
>>>>>Regards,
>>>>>Jakob
>>>>>
>>>>>
>>>>>2010/7/26 Rudy De Busscher <[email protected]>
>>>>>
>>>>>
>>>>>Hi all,
>>>>>>
>>>>>>I agree, duplicated code has to be avoided and when the 
>>>>>>maven-shade-plugin can 
>>>>
>>>>>>help, the better.
>>>>>>
>>>>>>but I think we have to define which project supplies code for which other 
>>>>>>project (so that we don't get a spaghetti (using the tomatoes ;) or get 
>>>>>>circular 
>>>>>>
>>>>>>dependencies.).  I know that MyFaces core exists much longer then MyFaces 
>>>>>>Test 
>>>>
>>>>>>but I find it more 
>>>>>>
>>>>>>
>>>>>>logic that the MyFaces Test supplies code for the MyFaces Core project.
>>>>>>
>>>>>>my 2c.
>>>>>>
>>>>>>Regards
>>>>>>Rudy
>>>>>>
>>>>>>
>>>>>>
>>>>>>On 26 July 2010 21:40, Matthias Wessendorf <[email protected]> wrote:
>>>>>>
>>>>>>On Mon, Jul 26, 2010 at 9:30 PM, Jakob Korherr <[email protected]> 
>>>>wrote:
>>>>>>>> Hehe, yeah I maybe forgot 10 "many".
>>>>>>>>
>>>>>>>> I can provide a wiki page for the general idea and the approach used on
>>>>>>>> myfaces-test.
>>>>>>>
>>>>>>>+1
>>>>>>>
>>>>>>>
>>>>>>>> Then everyone can adopt this idea and see how it works.
>>>>>>>
>>>>>>>great. I have that on my agenda as well, for Trinidad but a
>>>>>>>new release is more important, today...
>>>>>>>
>>>>>>>>
>>>>>>>> "RIP _shared? :)"
>>>>>>>> --> yes!
>>>>>>>
>>>>>>>I thought so. Yes! :)
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Jakob
>>>>>>>>
>>>>>>>> 2010/7/26 Matthias Wessendorf <[email protected]>
>>>>>>>>>
>>>>>>>>> On Mon, Jul 26, 2010 at 8:56 PM, Jakob Korherr 
><[email protected]>
>>>>>>>>> wrote:
>>>>>>>>> > Hi guys,
>>>>>>>>> >
>>>>>>>>> > Working on the tests for FlashImpl, I ran into a problem with the
>>>>>>>>> > AbstractAttributeMap (MYFACES-2840). After fixing it, I needed to 
>copy
>>>>>>>>> > my
>>>>>>>>> > changes over to myfaces-test to be able to use the new version in 
the
>>>>>>>>> > test
>>>>>>>>> > case (MYFACESTEST-21). And this copying really sucks...
>>>>>>>>>
>>>>>>>>> +1
>>>>>>>>>
>>>>>>>>> >
>>>>>>>>> > If you think about it there are many, many, many different places 
>>>>>>>>> > in 

>>>the
>>>>>>>>>
>>>>>>>>> you forgot a many :-)
>>>>>>>>>
>>>>>>>>> > whole MyFaces project where we have duplicate code, for example
>>>>>>>>> > package-private, unspecified classes on myfaces-api which also 
>>>>>>>>> > exist 

>>in
>>>>>>>>> > myfaces-impl, classes of myfaces-impl which are used for the mock
>>>>>>>>> > implementations of myfaces-test, or the biggest one: the shared 
>>>project.
>>>>>>>>> >
>>>>>>>>> > An introduction to the maven-shade-plugin: This plugin lets you use 
a
>>>>>>>>> > dependency to another project and then at build time it copies all
>>>>>>>>> > needed
>>>>>>>>> > classes to the created jar file, removes the dependency from the 
>>>pom.xml
>>>>>>>>> > and
>>>>>>>>> > changes the imports in the class files. Thus you work with the real
>>>>>>>>> > classes
>>>>>>>>> > at development time and then at build time the used classes are 
>copied
>>>>>>>>> > into
>>>>>>>>> > the artifact and the development dependency is gone. Furthermore 
>>>>>>>>> > you 

>>>can
>>>>>>>>> > make all kinds of alterations to those classes (e.g. change 
package).
>>>>>>>>> >
>>>>>>>>> > We successfully use the maven-shade-plugin in MyFaces CODI to reuse
>>>>>>>>> > functionaltiy between the JSF 1.2 and 2.0 modules. In addition, I 
>have
>>>>>>>>> > locally already used it to fix MYFACESTEST-21: I use it to get the
>>>>>>>>> > AbstractAttributeMap and all its related classes from myfaces-impl 
to
>>>>>>>>> > myfaces-test. And it really works like a calm.
>>>>>>>>> >
>>>>>>>>> > However I have to be honest, one thing currently has a bug: filters 
>>are
>>>>>>>>> > only
>>>>>>>>> > applied to the binary and not also to the sources jar (see
>>>>>>>>> > http://jira.codehaus.org/browse/MSHADE-83), but I already provided a
>>>>>>>>> > patch
>>>>>>>>> > and a test case for it, so I guess it will be fixed very soon.
>>>>>>>>> >
>>>>>>>>> > So if there are no objects, I would like to introduce the
>>>>>>>>> > maven-shade-plugin
>>>>>>>>> > on myfaces-test at first, to show you guys how awesome it is.
>>>>>>>>> > Afterwards,
>>>>>>>>> > and of course if you all like it, I would really like to use it on
>>>>>>>>> > several
>>>>>>>>> > places in MyFaces. This would for example be the chance to get rid 
of
>>>>>>>>> > the
>>>>>>>>> > shared project once and for all.
>>>>>>>>> >
>>>>>>>>> > Opinions, suggestions and tomatoes are welcome!
>>>>>>>>>
>>>>>>>>> +1 on this approach. I'd like to see the *adoption* (kinda)
>>>>>>>>> documented, so that other (sub) projects can *learn* from your 
efforts!
>>>>>>>>>
>>>>>>>>> RIP _shared? :)
>>>>>>>>>
>>>>>>>>> -Matthias
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> >
>>>>>>>>> > Regards,
>>>>>>>>> > Jakob
>>>>>>>>> >
>>>>>>>>> > --
>>>>>>>>> > Jakob Korherr
>>>>>>>>> >
>>>>>>>>> > blog: http://www.jakobk.com
>>>>>>>>> > twitter: http://twitter.com/jakobkorherr
>>>>>>>>> > work: http://www.irian.at
>>>>>>>>> >
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Matthias Wessendorf
>>>>>>>>>
>>>>>>>>> blog: http://matthiaswessendorf.wordpress.com/
>>>>>>>>> sessions: http://www.slideshare.net/mwessendorf
>>>>>>>>> twitter: http://twitter.com/mwessendorf
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Jakob Korherr
>>>>>>>>
>>>>>>>> blog: http://www.jakobk.com
>>>>>>>> twitter: http://twitter.com/jakobkorherr
>>>>>>>> work: http://www.irian.at
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>--
>>>>>>>
>>>>>>>Matthias Wessendorf
>>>>>>>
>>>>>>>blog: http://matthiaswessendorf.wordpress.com/
>>>>>>>sessions: http://www.slideshare.net/mwessendorf
>>>>>>>twitter: http://twitter.com/mwessendorf
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>-- 
>>>>>
>>>>>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
>


      

Reply via email to