[DISCUSS] how to get rid of tons of duplicated code

[Mark Struberg <st...@yahoo.de>]

Hi!


While working on the mafyces-commons cleanup I figured that we have tons of duplicated code spread over MyFaces. 



As an example I like to mention myfaces-commons-resourcehandler. There are 43 classes in total, and 35 of them are just 1:1 copied from other projects to provide resource management, zip, etc. For me this is an absolute no-go. Those classes have neither tests nor any documentation where they got forked from. Nor will any bug which gets fixed in another module make it's way over to all the other projects containing that very forked code. That's just ... unbelievable unmaintainable.  

There are 2 different ways to solve this (depending on the problem):

A.) drop the functionality and provide a generalized solution. The GZIP of myfaces-commons-resourcehandleris an obvious example:
We now copy this code over the 4th time or even more. Instead of doing this, we should rather do it in the classic unix fashion: do one thing, but do it well. 
Which
 means I'd rather see all the GZIP stuff factored out into an own 
mf-commons module as a Servlet Filter. This can then get applied to 
what ever other mechanism you like. This could also (commonly) cover 
cases like detecting http UserAgents which are not able to handle zipped
 resources, etc. That way we could provide this logic ONCE and have complete freedom over the configuration.

B.) code reusable components once and use them in other projects (ev via shading it in).
ClassLoaderResourceLoader would be a perfect candidate! I grepped through only the few pits which I 
have checked out locally and found this class 7 SEVEN times! I just 
can't believe that we can't move this stuff to a shared modul...

Same for FacesServletMapping. 6 times copied around, WebConfigProviderFactory 5 times, ...
There are whole packages with 10++ classes which got copied 1:1!

I really could cry seeing this :(


What can we do to solve this?

Theoretically myfaces-shared should contain this stuff. This is exactly what it is for!
Historically there have been some hand forged tweeks and ugly hacks, but nowadays we have the maven-shade-plugin to make our live easier.

So the suggestion is:

1.) cleanup myfaces-shared. mf-shared has almost no checkstyle rules applied. 
2.) add unit tests for myfaces-shared. Currently there are not many...
3.) move the shared code parts back to myfaces-shared and add unit tests. 
4.) import myfaces-shared via maven dependency and use <minimizeJar> and <relocations> to package the stuff 

[+1] fine go ahead (ideally: yes, what parts can I help with?)
[0] dont care
[-1] wont work because ...


I've attached a file which contains all Classes which name exists multiple times in MyFaces. The number is the cound how often they exist in MyFaces. I excluded current20.
Please note that classes with the same name do not necessarily have the same content - but quite a lot actually do have! (scroll to the bottom of the file ...)

LieGrue,
strub

Re: [DISCUSS] how to get rid of tons of duplicated code

[Mark Struberg <st...@yahoo.de>]

Leo, that's fine, but be aware that I'll go on unifying the codestyle over the next days.
So please be aware that you most likely will need to merge this branch manually.

LieGrue,
strub



----- Original Message -----
> From: Leonardo Uribe <lu...@gmail.com>
> To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg <st...@yahoo.de>
> Cc: 
> Sent: Tuesday, October 25, 2011 12:30 AM
> Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
> 
> Hi
> 
> I started a branch to try it:
> 
> http://svn.apache.org/repos/asf/myfaces/core/branches/2.0.x_refactor_shade_duplicate/
> 
> just to gain some time. I think it is worth at least to try a prototype.
> 
> regards,
> 
> Leonardo Uribe
> 
> 2011/10/24 Mark Struberg <st...@yahoo.de>:
>>  Hi Leo!
>> 
>>  Yes, this sounds much better, but please give me 2 days to think through it 
> in detail.
>> 
>>  txs and LieGrue,
>>  strub
>> 
>> 
>> 
>>  ----- Original Message -----
>>>  From: Leonardo Uribe <lu...@gmail.com>
>>>  To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg 
> <st...@yahoo.de>
>>>  Cc:
>>>  Sent: Monday, October 24, 2011 7:18 PM
>>>  Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>> 
>>>  Hi
>>> 
>>>  Ok, it is evident we have different opinions here about how to deal
>>>  with this problem. So, rather than refute the arguments I think it is
>>>  more productive to show another proposal. In MyFaces Core 2.0.x we
>>>  have the following layout:
>>> 
>>>  api/
>>>  assembly/
>>>  bundle/
>>>  impl/
>>>  implee6/
>>>  integration-tests/
>>>  pom.xml
>>>  shared/
>>>  src/
>>> 
>>>  Let's add two modules
>>> 
>>>  parent : contains the parent POM that all submodules should inherit.
>>>  shared-utils : utilities for JSF used in core, but built as an api.
>>> 
>>>  Now, we move the duplicate code related to myfaces-commons-utils into
>>>  this module, and shared will have shared-utils as dependency. impl
>>>  module will use maven-shade plugin to take the code from shared-utils
>>>  and shared (actually this is done for only for shared).
>>> 
>>>  When a release of myfaces-core is done, all modules are released, so
>>>  things go as usual. But have the parent as module has the advantage
>>>  that if we want to release shared-utils or shared internal modules, we
>>>  can do it only releasing parent (optional) and shared-utils.
>>> 
>>>  Since we can create a release for these modules, we remove the hack
>>>  used on on shared project (hard copy). Just we modify the pom to use
>>>  maven-shade-plugin over myfaces-core shared module. Then we kill
>>>  shared-tomahawk, and we make tomahawk use maven-shade-plugin over
>>>  shaded artifact in shared project.
>>> 
>>>  In commons we do the same as in shared. In
>>>  myfaces-commons-resourcehandler we use myfaces core shared module
>>>  using maven-shade-plugin.
>>> 
>>>  The only disadvantage is the release process gets a little bit more
>>>  complicated, but since do a release becomes easier with nexus, it is
>>>  ok.
>>> 
>>>  Do you agree with this solution?
>>> 
>>>  regards,
>>> 
>>>  Leonardo Uribe
>>> 
>>>  2011/10/24 Mark Struberg <st...@yahoo.de>:
>>>>>  Really that was not solved using maven-shade-plugin.
>>>>   Then we used the maven-shade-plugin the wrong way. See the
>>>  <relocations> option [1].
>>>> 
>>>>>   There, there is a profile called
>>>>>   "synch-myfaces-impl-shared", when it is added, the 
> code is
>>>  copied and
>>>>>   then a manual commit do the trick.
>>>> 
>>>>   I think this is an ugly hack and doesn't solve any problems 
> because
>>>> 
>>>>   a.)
>>>>>   Take into account
>>>>>   that each release requires a vote and that vote takes 3 days 
> to get
>>>>>   fixed.
>>>>   you could just do a mvn release of shared + core in 1 go to the 
> same
>>>  staging repo -> only 1 vote is needed!
>>>>   This argument is simply wrong.
>>>> 
>>>>   b.)
>>>>   You
>>>>    currently copy the code over 1:1 (half manually) thus your 
> argument
>>>>   with 'core and other projects need different sources' is 
> just nil.
>>>  There
>>>>    is no difference if you do this by profile, by hand or 
> automatically!
>>>>   So I really prefer to have this automatically. Which is exactly 
> what a
>>>>   dependency does...
>>>> 
>>>>   c.) the TCK argument is moot because it has
>>>>   nothing to do with shared. Most of the issues in the TCK are not
>>>>   affecting shared. And if they do, then those fixes are needed BY 
> ALL
>>>>   OTHER PROJECTS AS WELL. Thus another argument against hiding this 
> code
>>>>   and duplicating it all over...
>>>> 
>>>>   c.)
>>>>>   Instead, maybe the option is reorganize myfaces core to allow
>>>>>   alternate release lifecycles per module
>>>>   Sorry I don't grok this argument. It sounds as it would make 
> all things
>>>  more complicated without solving any real problem.
>>>> 
>>>>   e.)
>>>>>   This means myfaces-commons project should be 
> "merged" in some
>>>  way with
>>>>>   myfaces core. It has sense.
>>>>   2 options:
>>>>   1..) kill myfaces-shared completely and use the shared from core 
> instead.
>>>>   Downside: if you need some fix in the shared code for some other 
> project,
>>>  you would need to release mf-core
>>>>   2.) kill the newly introduced (this got only created a few weeks 
> back by
>>>  you) core-shared and use mf-shared again.
>>>>   Downside: hmmm nothing if one understands how to release 
> correctly.
>>>> 
>>>>   f.)
>>>>    all your explanations only explain the duplication between
>>>>   myfaces-shared and myfaces-core-shared. I can live with the 
> duplication
>>>>   for now, but we also have classes which got copied around up to 8 
> times!
>>>>    There is no excuse for that imo.
>>>> 
>>>>   g.)
>>>>>   But what happen when you have some code that does not have a 
> clear
>>>>>   "interface". If somebody removes or change some code 
> because
>>>  he/she
>>>>>   thinks it is not used in core or whatever, all 6 projects that 
> could
>>>>>   require it will be affected and will require to rework its 
> code.
>>>>>   Things get uglier when you have one library working with 
> version 1.1.1
>>>>>   and 1.1.2 is binary incompatible with version 1.1.1, but my 
> other
>>>>>   dependency requires it and kaboooom, the application does not 
> work.
>>>>>   So, the first assumption we need to preserve in those
>>>  "shared"
>>>>>   artifacts is build it as an API, preserving binary 
> compatibility.
>>>> 
>>>>   I don't get that argument neither!
>>>>   Hey,
>>>>    that's life! If it turns out that the code is not good enough 
> and
>>>  needs
>>>>    a fix, then that's the way it is! All other projects should 
> fix that
>>>>   too in that case. I rather have a reproducible compile error which
>>>>   easily could get fixed than having tons of duplicated code which 
> is more
>>>>    or less always logically broken and badly tested.
>>>>   Yes, we should be
>>>>   aware that the classes we put into myfaces-shared must meet some
>>>>   standards and need to be well tested. But actually that would 
> benefit
>>>>   our project a lot.
>>>> 
>>>> 
>>>>   h.) I just realised that our process in copying shared-impl from 
> core to
>>>  mf-shared is even more broken than every process before.
>>>>   If you are working on a lets say mf-commons project and find a bug 
> in any
>>>  of those shared parts, then you would need to RELEASE MF-CORE FIRST? 
> omg, this
>>>  cannot be serious!
>>>> 
>>>> 
>>>>   LieGrue,
>>>>   strub
>>>> 
>>>> 
>>>>   [1]
>>> 
> http://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#relocations
>>>> 
>>>> 
>>>> 
>>>>   ----- Original Message -----
>>>>>   From: Leonardo Uribe <lu...@gmail.com>
>>>>>   To: MyFaces Development <de...@myfaces.apache.org>; Mark 
> Struberg
>>>  <st...@yahoo.de>
>>>>>   Cc:
>>>>>   Sent: Monday, October 24, 2011 4:36 AM
>>>>>   Subject: Re: [DISCUSS] how to get rid of tons of duplicated 
> code
>>>>> 
>>>>>   Hi
>>>>> 
>>>>>   2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>>>>    I've now read through the old mail archives and 
> understand
>>>  what the
>>>>>   original problem was. But actually I don't think we solved 
> it
>>>  correctly
>>>>>   right now. Of course we solved to original problem, but opened 
> a can of
>>>  worms
>>>>>   causing other problems.
>>>>>> 
>>>>>>    The problem as far as I remember has been that 
> myfaces-shared had
>>>  tons of
>>>>>   duplicated code in it. One for core, one for tomahawk, one for
>>>  trinidad, etc.
>>>>>> 
>>>>>> 
>>>>>>    The shared part for core got moved to myfaces-core, but 
> the deeper
>>>  problem
>>>>>   was that it was not easily possible to have multiple different 
> versions
>>>  of
>>>>>   myfaces-shared. This now got solved by using the 
> maven-shade-plugin. So
>>>  we
>>>>>   should rethink the practice to duplicate all the code and aim 
> for a
>>>  _clean_
>>>>>   solution.
>>>>>> 
>>>>> 
>>>>>   Really that was not solved using maven-shade-plugin. What we 
> did was
>>>>>   copy the code into myfaces-core and create a mirror of the 
> same code
>>>>>   under shared. There, there is a profile called
>>>>>   "synch-myfaces-impl-shared", when it is added, the 
> code is
>>>  copied and
>>>>>   then a manual commit do the trick.
>>>>> 
>>>>>>    Also (being a maven guy) I cannot quite follow the 
> argument about
>>>  the
>>>>>   release cycles. Running a myfaces-shared release and then 
> (with the
>>>  same staging
>>>>>   repo) a myfaces-core release is a task of 15 minutes. + the 
> time for
>>>  running the
>>>>>   TCK, but this gets run via CI anyway, right? Thus this is 
> barely a
>>>  problem.
>>>>>>    If it is then I'd happily volunteer to do the next 
> release (do
>>>  this for
>>>>>   a few projects already) As you know, performing a release 
> really got
>>>  _much_
>>>>>   easier nowadays with our new apache-parent pom.
>>>>>>    But maybe this argument was only meant for our old 
> release process
>>>  (which I
>>>>>   agree was a lot of work)?
>>>>>> 
>>>>>>    If your answer is 'it's still needed' then 
> can we just
>>>  unify
>>>>>   all other usages?
>>>>>> 
>>>>> 
>>>>>   Make a release is just the first of the problems. Take into 
> account
>>>>>   that each release requires a vote and that vote takes 3 days 
> to get
>>>>>   fixed. So in practice a problem in core can effectively block 
> a
>>>>>   release of other artifacts. That's very inconvenient. 
> Suppose we
>>>  have
>>>>>   a new TCK and that one found a problem on myfaces core. Again 
> even if
>>>>>   the other artifacts are good enough, this becomes a blocker. 
> There are
>>>>>   enough historical evidence that supports this point. In 
> conclusion
>>>>>   this slow down the whole release cycle we have on myfaces. So 
> ignore
>>>>>   that is not an option.
>>>>> 
>>>>>   Instead, maybe the option is reorganize myfaces core to allow
>>>>>   alternate release lifecycles per module. For example, each 
> maven
>>>>>   plugin in myfaces has its own release lifecycle and there is a 
> parent
>>>>>   pom with a different release procedure. This requires some 
> changes to
>>>>>   create the source-release.zip file inherited from apache pom. 
> But it
>>>>>   could be a cleaner solution.
>>>>> 
>>>>>   This means myfaces-commons project should be 
> "merged" in some
>>>  way with
>>>>>   myfaces core. It has sense.
>>>>> 
>>>>>>    One question which bothers me with the 'shared' 
> approach
>>>  if what
>>>>>   would happen to our build-tools annotation scanning
>>>  (@JSFWebConfigParam, etc)?
>>>>>   Does this already work with dependencies? Do we have this 
> problem
>>>  already due to
>>>>>   the fact that we import such annotated classes via dependency?
>>>>>> 
>>>>> 
>>>>>   Those annotations comes from myfaces-builder-annotations. They 
> are
>>>>>   source code annotations but all that information are saved on
>>>>>   myfaces-metadata.xml, so even if dissapear on compile time, 
> the
>>>>>   information can be gathered from there. It is not a problem.
>>>>> 
>>>>>>>    Additionally, we increase the risk of "side
>>>  effects",
>>>>>>>    because a change done in core could introduce a bug 
> in other
>>>  parts.
>>>>>>    Imo it's exactly the opposite. If you use the same 
> code in 7
>>>  projects,
>>>>>   then it is more likely that a bug gets found and fixed.
>>>>>>    And the opposite case is (sadly) absolutely unlikely. If 
> you have
>>>  a class
>>>>>   duplicated 7 times and find a bug in one project, it is highly 
> unlikely
>>>  that all
>>>>>   6 other projects will get this fix applied :(
>>>>>> 
>>>>> 
>>>>>   But what happen when you have some code that does not have a 
> clear
>>>>>   "interface". If somebody removes or change some code 
> because
>>>  he/she
>>>>>   thinks it is not used in core or whatever, all 6 projects that 
> could
>>>>>   require it will be affected and will require to rework its 
> code.
>>>>>   Things get uglier when you have one library working with 
> version 1.1.1
>>>>>   and 1.1.2 is binary incompatible with version 1.1.1, but my 
> other
>>>>>   dependency requires it and kaboooom, the application does not 
> work.
>>>>>   So, the first assumption we need to preserve in those
>>>  "shared"
>>>>>   artifacts is build it as an API, preserving binary 
> compatibility.
>>>>> 
>>>>>   So we can't just grab the code from shared as is and say 
> to users
>>>  "...
>>>>>   you can use that into its own projects ...". If the 
> project is
>>>>>   maintained inside myfaces we can fix such problems, but 
> outside
>>>>>   myfaces we should be more strict. So, we need a "public
>>>  shared" code
>>>>>   like the one proposed in myfaces commons and other code 
> "myfaces
>>>>>   shared" to use in projects like tomahawk or portletbridge 
> or
>>>  whatever
>>>>>   inside our land.
>>>>> 
>>>>>   regards,
>>>>> 
>>>>>   Leonardo Uribe
>>>>> 
>>>>>>    LieGrue,
>>>>>>    strub
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>>    ----- Original Message -----
>>>>>>>    From: Leonardo Uribe <lu...@gmail.com>
>>>>>>>    To: MyFaces Development 
> <de...@myfaces.apache.org>
>>>>>>>    Cc: Mark Struberg <st...@yahoo.de>
>>>>>>>    Sent: Sunday, October 23, 2011 9:08 PM
>>>>>>>    Subject: Re: [DISCUSS] how to get rid of tons of 
> duplicated
>>>  code
>>>>>>> 
>>>>>>>    Hi
>>>>>>> 
>>>>>>>    Ok, let's check the proposal
>>>>>>> 
>>>>>>>    MS>> So the suggestion is:
>>>>>>>    MS>>
>>>>>>>    MS>> 1.) cleanup myfaces-shared. mf-shared has 
> almost no
>>>>>   checkstyle
>>>>>>>    rules applied.
>>>>>>> 
>>>>>>>    Yes, sounds good.
>>>>>>> 
>>>>>>>    MS>> 2.) add unit tests for myfaces-shared. 
> Currently
>>>  there are
>>>>>   not
>>>>>>>    many...
>>>>>>> 
>>>>>>>    Ok, sounds good too.
>>>>>>> 
>>>>>>>    MS>> 3.) move the shared code parts back to
>>>  myfaces-shared and
>>>>>   add unit
>>>>>>>    tests.
>>>>>>> 
>>>>>>>    So, this means do one step back and move the code 
> from
>>>  myfaces-core
>>>>>>>    "shared" to myfaces-shared project? This 
> breaks
>>>  effectively
>>>>>   the
>>>>>>>    changes done some months ago to make easier work with 
> myfaces
>>>  core
>>>>>>>    itself.
>>>>>>> 
>>>>>>>    In that time the conclusion was: "core has 
> priority over
>>>  anything
>>>>>>>    else, so shared code must live in core, but 
> myfaces-shared
>>>  project
>>>>>>>    should just copy the code from there and have its own
>>>  lifecycle"
>>>>>>>    (these are my own words as I understood).
>>>>>>> 
>>>>>>>    So this point does not have practical sense, and go 
> against
>>>  everything
>>>>>>>    discussed earlier.
>>>>>>> 
>>>>>>>    MS>> 4.) import myfaces-shared via maven 
> dependency and
>>>  use
>>>>>>>    <minimizeJar> and <relocations> to 
> package the
>>>  stuff
>>>>>>> 
>>>>>>>    maven-shade-plugin is a good "tool" but 
> doesn't
>>>  fit well
>>>>>   in this
>>>>>>>    scenario. The reason is we need an alternate release 
> lifecycle
>>>  for the
>>>>>>>    shared code between myfaces core and other projects.
>>>>>>> 
>>>>>>>    Historically that was the very first intention behind
>>>  myfaces-shared
>>>>>>>    project. Any myfaces core release requires some 
> additional
>>>  steps to do
>>>>>>>    (TCK), so that becomes a problem when you try to 
> release other
>>>>>>>    libraries that depends of shared. So, to fix that,
>>>  "shared"
>>>>>   was
>>>>>>>    created, so the code can be released in a independent 
> way, and
>>>  prevent
>>>>>>>    myfaces core becomes an obstacle to release any other 
> project
>>>>>>>    (tomahawk, portlet-bridge, ... ). So, to release 
> tomahawk you
>>>  release
>>>>>>>    shared first and then tomahawk.
>>>>>>> 
>>>>>>>    maven-shade-plugin requires a released artifact to do 
> its job.
>>>  So, use
>>>>>>>    it impose that restriction. In "shared" 
> case,
>>>  preserve the
>>>>>   original
>>>>>>>    intention becomes "imperative", and 
> that's the
>>>  reason why
>>>>>   a goal
>>>>>>>    was
>>>>>>>    created to copy the code from myfaces-core shared, so 
> the
>>>  release
>>>>>>>    manager can run this goal, commit the changes and 
> then run a
>>>  release.
>>>>>>> 
>>>>>>>    My proposal in this case is do the same we did for 
> shared, but
>>>  for
>>>>>>>    "myfaces commons" case. Then we can use
>>>  maven-shade-plugin in
>>>>>   other
>>>>>>>    projects, but not over shared, instead over a 
> released version
>>>  of
>>>>>>>    myfaces-commons-utils. Keep tomahawk or 
> portlet-bridge as is,
>>>  using
>>>>>>>    shared project, because by its nature, those projects 
> require
>>>  classes
>>>>>>>    that are not meant to be used outside those cases.
>>>>>>> 
>>>>>>>    Note do any hack in this part makes a little bit
>>>  "obscure"
>>>>>   how to make
>>>>>>>    changes, because everything becomes 
> "centralized",
>>>  but makes
>>>>>   easier
>>>>>>>    maintain code. Additionally, we increase the risk of
>>>  "side
>>>>>   effects",
>>>>>>>    because a change done in core could introduce a bug 
> in other
>>>  parts. So
>>>>>>>    at the end this is a matter of how to keep our code
>>>>>   "balanced", even
>>>>>>>    if some times it becomes a decision about 
> "choose the
>>>  less
>>>>>>>    inconvenient alternative".
>>>>>>> 
>>>>>>>    regards,
>>>>>>> 
>>>>>>>    Leonardo Uribe
>>>>>>> 
>>>>>>>    2011/10/23 Leonardo Uribe <lu...@gmail.com>:
>>>>>>>>     Hi
>>>>>>>> 
>>>>>>>>     2011/10/23 Jakob Korherr 
> <ja...@gmail.com>:
>>>>>>>>>     Hi Mark,
>>>>>>>>> 
>>>>>>>>>     +1 - that's exactly what I have been 
> trying to
>>>  accomplish
>>>>>   some time
>>>>>>>>>     ago (introducing common-shades [1]). 
> Unfortunately, I
>>>  was not
>>>>>>>>>     successful back then.
>>>>>>>>> 
>>>>>>>> 
>>>>>>>>     It is clear we need to "split" 
> myfaces-impl
>>>  into
>>>>>   multiple
>>>>>>>    modules. There
>>>>>>>>     are some parts that are useful for other 
> projects. The
>>>  code you
>>>>>   did
>>>>>>>>     on commons-shade was the attempt to solve the 
> problem of
>>>  the
>>>>>>>>     duplicate code used on myfaces-test.
>>>>>>>> 
>>>>>>>>     Now the objective is find a way about how to 
> reuse code
>>>  in myfaces
>>>>>>>>     core between multiple projects effectively.
>>>>>>>> 
>>>>>>>>>     However, there is a slight problem with 
> moving all
>>>  this stuff
>>>>>   into
>>>>>>>>>     MyFaces shared, which I want to point out: 
> code size.
>>>  If we
>>>>>   really put
>>>>>>>>>     all the code that is shared across any 
> MyFaces
>>>  subproject into
>>>>>   shared,
>>>>>>>>>     it will get fat and ugly (even more than it 
> is right
>>>  now). In
>>>>>>>>>     addition, if we continue including the whole 
> shared
>>>  project
>>>>>   into
>>>>>>>>>     MyFaces core, MyFaces core impl will get 
> bigger and
>>>  bigger.
>>>>>>>>> 
>>>>>>>> 
>>>>>>>>     Yes, the problem basically is MyFaces shared 
> does not
>>>  have any
>>>>>   order
>>>>>>>>     or any notion of API. There are code that is 
> used only in
>>>  tomahawk
>>>>>   but not
>>>>>>>>     intended to use in any other place. There are 
> some useful
>>>>>   utitlities but
>>>>>>>>     sometimes without documentation, and there are 
> some other
>>>  code
>>>>>   that is
>>>>>>>>     just obsolete. It it clear a cleanup of that 
> location is
>>>>>>>>     necessary, but note priorities comes first, so 
> this task
>>>  has been
>>>>>   delayed
>>>>>>>    in
>>>>>>>>     order to deal with other important stuff. Now it 
> is a
>>>  good time to
>>>>>   fix
>>>>>>>    this.
>>>>>>>> 
>>>>>>>>>     Thus I'd like to suggest something 
> similar which
>>>  I wanted
>>>>>   to
>>>>>>>>>     accomplish with common-shades: Introduce a 
> new shared
>>>  module,
>>>>>   which
>>>>>>>>>     consists of many submodules that each handle 
> a
>>>  specific
>>>>>   functionality
>>>>>>>>>     instead of being one fat module. With this 
> approach
>>>  each
>>>>>   MyFaces
>>>>>>>>>     subproject would be able to pick out only 
> the stuff
>>>  it really
>>>>>   needs.
>>>>>>>>>     Furthermore we would see more easily which 
> code in
>>>  shared is
>>>>>   not used
>>>>>>>>>     anymore (I guess at the moment there is a 
> lot of it),
>>>  just by
>>>>>   checking
>>>>>>>>>     which modules are still used in our poms.
>>>>>>>>> 
>>>>>>>> 
>>>>>>>>     That is the big question, how to split 
> myfaces-impl and
>>>  shared.
>>>>>   Precisely
>>>>>>>>     the intention of myfaces-commons-utils projects 
> was take
>>>  the stuff
>>>>>   that is
>>>>>>>>     useful from shared and build an usable API for 
> developers
>>>  outside
>>>>>   MyFaces.
>>>>>>>> 
>>>>>>>>     For example, MyFaces HTML5 subproject was a good
>>>  experiment to see
>>>>>>>>     which code is useful and should be added in a 
> API. Some
>>>  weeks ago
>>>>>   I checked
>>>>>>>>     and removed all duplicate code to use
>>>  myfaces-commons-utils. So
>>>>>   the 1.0.2
>>>>>>>>     release contains those classes taken from 
> shared.
>>>>>>>> 
>>>>>>>>     regards,
>>>>>>>> 
>>>>>>>>     Leonardo Uribe
>>>>>>>> 
>>>>>>>>>     Regards,
>>>>>>>>>     Jakob
>>>>>>>>> 
>>>>>>>>>     [1]
>>>  https://svn.apache.org/repos/asf/myfaces/common-shades/
>>>>>>>>> 
>>>>>>>>>     2011/10/23 Mark Struberg 
> <st...@yahoo.de>:
>>>>>>>>>>     Hi!
>>>>>>>>>>     While working on the mafyces-commons 
> cleanup I
>>>  figured
>>>>>   that we have
>>>>>>>    tons of
>>>>>>>>>>     duplicated code spread over MyFaces.
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>>     As an example I like to mention
>>>>>   myfaces-commons-resourcehandler.
>>>>>>>    There are
>>>>>>>>>>     43 classes in total, and 35 of them are 
> just 1:1
>>>  copied
>>>>>   from other
>>>>>>>    projects
>>>>>>>>>>     to provide resource management, zip, 
> etc. For me
>>>  this is
>>>>>   an
>>>>>>>    absolute no-go.
>>>>>>>>>>     Those classes have neither tests nor any
>>>  documentation
>>>>>   where they
>>>>>>>    got forked
>>>>>>>>>>     from. Nor will any bug which gets fixed 
> in
>>>  another module
>>>>>   make
>>>>>>>    it's way over
>>>>>>>>>>     to all the other projects containing 
> that very
>>>  forked
>>>>>   code.
>>>>>>>    That's just ...
>>>>>>>>>>     unbelievable unmaintainable.
>>>>>>>>>> 
>>>>>>>>>>     There are 2 different ways to solve this
>>>  (depending on the
>>>>>>>    problem):
>>>>>>>>>> 
>>>>>>>>>>     A.) drop the functionality and provide a
>>>  generalized
>>>>>   solution. The
>>>>>>>    GZIP of
>>>>>>>>>>     myfaces-commons-resourcehandleris an 
> obvious
>>>  example:
>>>>>>>>>>     We now copy this code over the 4th time 
> or even
>>>  more.
>>>>>   Instead of
>>>>>>>    doing this,
>>>>>>>>>>     we should rather do it in the classic 
> unix
>>>  fashion: do one
>>>>>   thing,
>>>>>>>    but do it
>>>>>>>>>>     well.
>>>>>>>>>>     Which means I'd rather see all the 
> GZIP stuff
>>>  factored
>>>>>   out into
>>>>>>>    an own
>>>>>>>>>>     mf-commons module as a Servlet Filter. 
> This can
>>>  then get
>>>>>   applied to
>>>>>>>    what
>>>>>>>>>>     ever other mechanism you like. This 
> could also
>>>  (commonly)
>>>>>   cover
>>>>>>>    cases like
>>>>>>>>>>     detecting http UserAgents which are not 
> able to
>>>  handle
>>>>>   zipped
>>>>>>>    resources,
>>>>>>>>>>     etc. That way we could provide this 
> logic ONCE
>>>  and have
>>>>>   complete
>>>>>>>    freedom
>>>>>>>>>>     over the configuration.
>>>>>>>>>> 
>>>>>>>>>>     B.) code reusable components once and 
> use them in
>>>  other
>>>>>   projects
>>>>>>>    (ev via
>>>>>>>>>>     shading it in).
>>>>>>>>>>     ClassLoaderResourceLoader would be a 
> perfect
>>>  candidate! I
>>>>>   grepped
>>>>>>>    through
>>>>>>>>>>     only the few pits which I have checked 
> out
>>>  locally and
>>>>>   found this
>>>>>>>    class 7
>>>>>>>>>>     SEVEN times! I just can't believe 
> that we
>>>  can't
>>>>>   move this
>>>>>>>    stuff to a shared
>>>>>>>>>>     modul...
>>>>>>>>>> 
>>>>>>>>>>     Same for FacesServletMapping. 6 times 
> copied
>>>  around,
>>>>>>>>>>     WebConfigProviderFactory 5 times, ...
>>>>>>>>>>     There are whole packages with 10++ 
> classes which
>>>  got
>>>>>   copied 1:1!
>>>>>>>>>> 
>>>>>>>>>>     I really could cry seeing this :(
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>>     What can we do to solve this?
>>>>>>>>>> 
>>>>>>>>>>     Theoretically myfaces-shared should 
> contain this
>>>  stuff.
>>>>>   This is
>>>>>>>    exactly what
>>>>>>>>>>     it is for!
>>>>>>>>>>     Historically there have been some hand 
> forged
>>>  tweeks and
>>>>>   ugly
>>>>>>>    hacks, but
>>>>>>>>>>     nowadays we have the maven-shade-plugin 
> to make
>>>  our live
>>>>>   easier.
>>>>>>>>>> 
>>>>>>>>>>     So the suggestion is:
>>>>>>>>>> 
>>>>>>>>>>     1.) cleanup myfaces-shared. mf-shared 
> has almost
>>>  no
>>>>>   checkstyle
>>>>>>>    rules
>>>>>>>>>>     applied.
>>>>>>>>>>     2.) add unit tests for myfaces-shared. 
> Currently
>>>  there are
>>>>>   not
>>>>>>>    many...
>>>>>>>>>>     3.) move the shared code parts back to
>>>  myfaces-shared and
>>>>>   add unit
>>>>>>>    tests.
>>>>>>>>>>     4.) import myfaces-shared via maven 
> dependency
>>>  and use
>>>>>>>    <minimizeJar> and
>>>>>>>>>>     <relocations> to package the stuff
>>>>>>>>>> 
>>>>>>>>>>     [+1] fine go ahead (ideally: yes, what 
> parts can
>>>  I help
>>>>>   with?)
>>>>>>>>>>     [0] dont care
>>>>>>>>>>     [-1] wont work because ...
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>>     I've attached a file which contains 
> all
>>>  Classes which
>>>>>   name
>>>>>>>    exists multiple
>>>>>>>>>>     times in MyFaces. The number is the 
> cound how
>>>  often they
>>>>>   exist in
>>>>>>>    MyFaces. I
>>>>>>>>>>     excluded current20.
>>>>>>>>>>     Please note that classes with the same 
> name do
>>>  not
>>>>>   necessarily have
>>>>>>>    the same
>>>>>>>>>>     content - but quite a lot actually do 
> have!
>>>  (scroll to the
>>>>>   bottom
>>>>>>>    of the
>>>>>>>>>>     file ...)
>>>>>>>>>> 
>>>>>>>>>>     LieGrue,
>>>>>>>>>>     strub
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>>     --
>>>>>>>>>     Jakob Korherr
>>>>>>>>> 
>>>>>>>>>     blog: http://www.jakobk.com
>>>>>>>>>     twitter: http://twitter.com/jakobkorherr
>>>>>>>>>     work: http://www.irian.at
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>

Re: [DISCUSS] how to get rid of tons of duplicated code

[Mark Struberg <st...@yahoo.de>]

Hi Leo!

ad 1.

> To allow release shared-utils or shared module without the burden to
> release all myfaces core.
we don't have any problems with the current structure neither.
go to the myfaces/core/shared module and change the <parent> to the latest released one (usually it is on the current SNAPSHOT). then release with a -mt1 etc postfix and you are done. After the release you can just point the <parent> to the SNAPSHOT again.

ad 2.
> It means code there should documented, tested, should preserve binary
> compatibility against backward versions on the same artifact, and
Oki, so you are talking about a
* shared-internal
and 

* shared-public or shared-stable

btw, I really dont know why all those modules are named 'shared' and are in different modules .

A jar is by definition a 'shared reusable component' ;)

ad 3.
>> +1 to dropping myfaces-commons-utils.
> I already sync the code available in myfaces-commons-utils with the
> duplicate code in shared, using a diff tool.
cool, thanks!


> The next step is use
> maven-shade-plugin to grab the code of the shared-utils internal
I can do this if you like. 


ad 5.
> But those projects can use myfaces-core shared internal module
> directly, so myfaces/shared is no longer necessary.
yup +1 here too. 

> The benefit of keep things at myfaces/core is it simplify testing.
> Before if a bug in shared was found, it was required to build shared,
> core and the test webapp.
Yes +1, that's a really strong argument!


> In practice, with this
> proposal we are only putting things in order, but we are not changing
> the 'substance'.
Yes, hopefully the various Classes which got copied around are still the same and no 'fix' or hack got added to one of them.
The major benefit from a user perspective is that some person who checks out the code only sees the code which is really the core of his application, and not gets 5-times more classes which just got copied around.

In case of the myfaces-commons-resourcehandler which I like to use, it makes the difference between 9 classes and 43 - that's actually a big deal for me ;)

LieGrue,
strub


----- Original Message -----
> From: Leonardo Uribe <lu...@gmail.com>
> To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg <st...@yahoo.de>
> Cc: 
> Sent: Tuesday, October 25, 2011 5:48 PM
> Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
> 
> 2011/10/25 Mark Struberg <st...@yahoo.de>:
>>  Oki, just found time to do a  review:
>> 
>>  1.) why do we need an own 'parent' module? I think all the basic 
> plugin definitions should stay the same for all myfaces-core project, don't 
> they?
>> 
> 
> To allow release shared-utils or shared module without the burden to
> release all myfaces core. For example, if the last release was 2.1.4,
> we can release shared internal module with version number 2.1.4.1. The
> pom on 'parent' module has all base definitions that should stay the
> same for all myfaces-core project, so things have not changed, just
> think about this as a simple file relocation, as simple as that.
> 
>>  2.)
>>>  shared-utils : utilities for JSF used in core, but built as an api.
>>  what do you mean with 'built as an api' ?
>> 
> 
> It means code there should documented, tested, should preserve binary
> compatibility against backward versions on the same artifact, and
> should be useful outside myfaces projects. In other words, code there
> has a clear "interface". 'shared' module continue working as 
> usual,
> just a place to share code with tomahawk and other internal myfaces
> projects. The idea is if we create some utility on shared and we found
> it is good enough later we move it to shared-utils, and at the end do
> a cleanup over all shared code.
> 
>> 
>>  3.)
>>>  Now, we move the duplicate code related to myfaces-commons-utils
>> 
>>>  into this module
>>  +1 to dropping myfaces-commons-utils. This is just a 1:1 fork from shared.
>>  Could you please explain again where you like to move/copy this parts to 
> now?
>> 
> 
> I already sync the code available in myfaces-commons-utils with the
> duplicate code in shared, using a diff tool. The next step is use
> maven-shade-plugin to grab the code of the shared-utils internal
> module and do a package rename, removing the duplicate code from
> myfaces-commons-utils. In practice, it should only have some resource
> files related to i18n, but no java files (because those files will be
> maintained from shared-utils).
> 
>>  4.)
>>>  and shared will have shared-utils as dependency
>>  Which 'shared-utils' do you mean? the newly proposed module 
> myfaces/core/shared ?
>>  Actually myfaces/shared is then not needed anymore, right? This would be a 
> pretty big change to what has been.
>> 
>>  To be honest: I'd rather make the unique source of all those things in 
> myfaces/shared which it has been for years until only recently as this only got 
> changed in July or August this year! I really just see no benefit in keeping 
> this at myfaces/core/ over having it well maintained in myfaces/shared.
>>  If we do that, we also don't need any ugly hacks while doing async 
> releases - that's exactly what such a library is designed for
>> 
> 
> Look that shared uses maven-shade-plugin to include shared-utils into
> its jar. Tomahawk and other projects still needs shared module as is.
> But those projects can use myfaces-core shared internal module
> directly, so myfaces/shared is no longer necessary.
> 
> The benefit of keep things at myfaces/core is it simplify testing.
> Before if a bug in shared was found, it was required to build shared,
> core and the test webapp. With these change you only need to build
> core and the test webapp. This was the major complain about shared,
> and the reason why most people wanted to remove it.
> 
> The release procedure is not a problem with the new strategy proposed,
> because this one has been widely tested before and really don't
> enforce any hack at all. All magic was done in the pom relocation to
> parent module and other small changes.
> 
>>  5.)
>>>  Then we kill shared-tomahawk, and we make tomahawk use
>>>  maven-shade-plugin ..
>> 
>>   +1
>>  Actually this will be the biggest amount of work. To get rid of and unify 
> all the copied sources again.
>>  But I really think this is way worth it!
>> 
>> 
>>  Summary:
>> 
>>  I think we now agree that we should aim to 'unify' our code which 
> got copied multiple times and pull them back to a common usable library, right?
>> 
>>  We are not yet d'accord if this common library should reside in 
> myfaces/core/shared (as part of MyFaces core) or myfaces/shared (as a separate 
> MyFaces subproject). What about listing pros and cons and then start a vote?
>> 
> 
> As is considered, myfaces/core/shared and myfaces/core/shared-utils
> are internal modules. myfaces-commons-utils library just take
> shared-utils, repackage and makes it available to public. At the end,
> the end user will never see the difference. In practice, with this
> proposal we are only putting things in order, but we are not changing
> the 'substance'.
> 
> regards,
> 
> Leonardo U
> 
>> 
>>  txs and LieGrue,
>>  strub
>> 
>> 
>>  PS: no worry, together we will soon solve this riddle ;)
>> 
>> 
>> 
>>  ----- Original Message -----
>>>  From: Leonardo Uribe <lu...@gmail.com>
>>>  To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg 
> <st...@yahoo.de>
>>>  Cc:
>>>  Sent: Tuesday, October 25, 2011 4:02 AM
>>>  Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>> 
>>>  Hi
>>> 
>>>  Ok, it is done. In the branch it is possible to see how core will look
>>>  after the change. The other changes are just use maven-shade plugin
>>>  over myfaces-impl-shared-utils and myfaces-impl-shared internal
>>>  modules, and do some package relocation in some cases.
>>> 
>>>  regards
>>> 
>>>  Leonardo Uribe
>>> 
>>>  2011/10/24 Leonardo Uribe <lu...@gmail.com>:
>>>>   Hi
>>>> 
>>>>   I started a branch to try it:
>>>> 
>>>> 
>>> 
> http://svn.apache.org/repos/asf/myfaces/core/branches/2.0.x_refactor_shade_duplicate/
>>>> 
>>>>   just to gain some time. I think it is worth at least to try a 
> prototype.
>>>> 
>>>>   regards,
>>>> 
>>>>   Leonardo Uribe
>>>> 
>>>>   2011/10/24 Mark Struberg <st...@yahoo.de>:
>>>>>   Hi Leo!
>>>>> 
>>>>>   Yes, this sounds much better, but please give me 2 days to 
> think
>>>  through it in detail.
>>>>> 
>>>>>   txs and LieGrue,
>>>>>   strub
>>>>> 
>>>>> 
>>>>> 
>>>>>   ----- Original Message -----
>>>>>>   From: Leonardo Uribe <lu...@gmail.com>
>>>>>>   To: MyFaces Development <de...@myfaces.apache.org>; 
> Mark
>>>  Struberg <st...@yahoo.de>
>>>>>>   Cc:
>>>>>>   Sent: Monday, October 24, 2011 7:18 PM
>>>>>>   Subject: Re: [DISCUSS] how to get rid of tons of 
> duplicated code
>>>>>> 
>>>>>>   Hi
>>>>>> 
>>>>>>   Ok, it is evident we have different opinions here about 
> how to deal
>>>>>>   with this problem. So, rather than refute the arguments I 
> think it
>>>  is
>>>>>>   more productive to show another proposal. In MyFaces Core 
> 2.0.x we
>>>>>>   have the following layout:
>>>>>> 
>>>>>>   api/
>>>>>>   assembly/
>>>>>>   bundle/
>>>>>>   impl/
>>>>>>   implee6/
>>>>>>   integration-tests/
>>>>>>   pom.xml
>>>>>>   shared/
>>>>>>   src/
>>>>>> 
>>>>>>   Let's add two modules
>>>>>> 
>>>>>>   parent : contains the parent POM that all submodules 
> should
>>>  inherit.
>>>>>>   shared-utils : utilities for JSF used in core, but built 
> as an api.
>>>>>> 
>>>>>>   Now, we move the duplicate code related to 
> myfaces-commons-utils
>>>  into
>>>>>>   this module, and shared will have shared-utils as 
> dependency. impl
>>>>>>   module will use maven-shade plugin to take the code from
>>>  shared-utils
>>>>>>   and shared (actually this is done for only for shared).
>>>>>> 
>>>>>>   When a release of myfaces-core is done, all modules are 
> released,
>>>  so
>>>>>>   things go as usual. But have the parent as module has the 
> advantage
>>>>>>   that if we want to release shared-utils or shared internal 
> modules,
>>>  we
>>>>>>   can do it only releasing parent (optional) and 
> shared-utils.
>>>>>> 
>>>>>>   Since we can create a release for these modules, we remove 
> the hack
>>>>>>   used on on shared project (hard copy). Just we modify the 
> pom to
>>>  use
>>>>>>   maven-shade-plugin over myfaces-core shared module. Then 
> we kill
>>>>>>   shared-tomahawk, and we make tomahawk use 
> maven-shade-plugin over
>>>>>>   shaded artifact in shared project.
>>>>>> 
>>>>>>   In commons we do the same as in shared. In
>>>>>>   myfaces-commons-resourcehandler we use myfaces core shared 
> module
>>>>>>   using maven-shade-plugin.
>>>>>> 
>>>>>>   The only disadvantage is the release process gets a little 
> bit more
>>>>>>   complicated, but since do a release becomes easier with 
> nexus, it
>>>  is
>>>>>>   ok.
>>>>>> 
>>>>>>   Do you agree with this solution?
>>>>>> 
>>>>>>   regards,
>>>>>> 
>>>>>>   Leonardo Uribe
>>>>>> 
>>>>>>   2011/10/24 Mark Struberg <st...@yahoo.de>:
>>>>>>>>   Really that was not solved using 
> maven-shade-plugin.
>>>>>>>    Then we used the maven-shade-plugin the wrong way. 
> See the
>>>>>>   <relocations> option [1].
>>>>>>> 
>>>>>>>>    There, there is a profile called
>>>>>>>>    "synch-myfaces-impl-shared", when it is 
> added,
>>>  the code is
>>>>>>   copied and
>>>>>>>>    then a manual commit do the trick.
>>>>>>> 
>>>>>>>    I think this is an ugly hack and doesn't solve 
> any
>>>  problems because
>>>>>>> 
>>>>>>>    a.)
>>>>>>>>    Take into account
>>>>>>>>    that each release requires a vote and that vote 
> takes 3
>>>  days to get
>>>>>>>>    fixed.
>>>>>>>    you could just do a mvn release of shared + core in 1 
> go to
>>>  the same
>>>>>>   staging repo -> only 1 vote is needed!
>>>>>>>    This argument is simply wrong.
>>>>>>> 
>>>>>>>    b.)
>>>>>>>    You
>>>>>>>     currently copy the code over 1:1 (half manually) 
> thus your
>>>  argument
>>>>>>>    with 'core and other projects need different 
> sources'
>>>  is just nil.
>>>>>>   There
>>>>>>>     is no difference if you do this by profile, by hand 
> or
>>>  automatically!
>>>>>>>    So I really prefer to have this automatically. Which 
> is
>>>  exactly what a
>>>>>>>    dependency does...
>>>>>>> 
>>>>>>>    c.) the TCK argument is moot because it has
>>>>>>>    nothing to do with shared. Most of the issues in the 
> TCK are
>>>  not
>>>>>>>    affecting shared. And if they do, then those fixes 
> are needed
>>>  BY ALL
>>>>>>>    OTHER PROJECTS AS WELL. Thus another argument against 
> hiding
>>>  this code
>>>>>>>    and duplicating it all over...
>>>>>>> 
>>>>>>>    c.)
>>>>>>>>    Instead, maybe the option is reorganize myfaces 
> core to
>>>  allow
>>>>>>>>    alternate release lifecycles per module
>>>>>>>    Sorry I don't grok this argument. It sounds as it 
> would
>>>  make all things
>>>>>>   more complicated without solving any real problem.
>>>>>>> 
>>>>>>>    e.)
>>>>>>>>    This means myfaces-commons project should be
>>>  "merged" in some
>>>>>>   way with
>>>>>>>>    myfaces core. It has sense.
>>>>>>>    2 options:
>>>>>>>    1..) kill myfaces-shared completely and use the 
> shared from
>>>  core instead.
>>>>>>>    Downside: if you need some fix in the shared code for 
> some
>>>  other project,
>>>>>>   you would need to release mf-core
>>>>>>>    2.) kill the newly introduced (this got only created 
> a few
>>>  weeks back by
>>>>>>   you) core-shared and use mf-shared again.
>>>>>>>    Downside: hmmm nothing if one understands how to 
> release
>>>  correctly.
>>>>>>> 
>>>>>>>    f.)
>>>>>>>     all your explanations only explain the duplication 
> between
>>>>>>>    myfaces-shared and myfaces-core-shared. I can live 
> with the
>>>  duplication
>>>>>>>    for now, but we also have classes which got copied 
> around up
>>>  to 8 times!
>>>>>>>     There is no excuse for that imo.
>>>>>>> 
>>>>>>>    g.)
>>>>>>>>    But what happen when you have some code that does 
> not have
>>>  a clear
>>>>>>>>    "interface". If somebody removes or 
> change some
>>>  code because
>>>>>>   he/she
>>>>>>>>    thinks it is not used in core or whatever, all 6 
> projects
>>>  that could
>>>>>>>>    require it will be affected and will require to 
> rework its
>>>  code.
>>>>>>>>    Things get uglier when you have one library 
> working with
>>>  version 1.1.1
>>>>>>>>    and 1.1.2 is binary incompatible with version 
> 1.1.1, but
>>>  my other
>>>>>>>>    dependency requires it and kaboooom, the 
> application does
>>>  not work.
>>>>>>>>    So, the first assumption we need to preserve in 
> those
>>>>>>   "shared"
>>>>>>>>    artifacts is build it as an API, preserving 
> binary
>>>  compatibility.
>>>>>>> 
>>>>>>>    I don't get that argument neither!
>>>>>>>    Hey,
>>>>>>>     that's life! If it turns out that the code is 
> not good
>>>  enough and
>>>>>>   needs
>>>>>>>     a fix, then that's the way it is! All other 
> projects
>>>  should fix that
>>>>>>>    too in that case. I rather have a reproducible 
> compile error
>>>  which
>>>>>>>    easily could get fixed than having tons of duplicated 
> code
>>>  which is more
>>>>>>>     or less always logically broken and badly tested.
>>>>>>>    Yes, we should be
>>>>>>>    aware that the classes we put into myfaces-shared 
> must meet
>>>  some
>>>>>>>    standards and need to be well tested. But actually 
> that would
>>>  benefit
>>>>>>>    our project a lot.
>>>>>>> 
>>>>>>> 
>>>>>>>    h.) I just realised that our process in copying 
> shared-impl
>>>  from core to
>>>>>>   mf-shared is even more broken than every process before.
>>>>>>>    If you are working on a lets say mf-commons project 
> and find a
>>>  bug in any
>>>>>>   of those shared parts, then you would need to RELEASE 
> MF-CORE
>>>  FIRST? omg, this
>>>>>>   cannot be serious!
>>>>>>> 
>>>>>>> 
>>>>>>>    LieGrue,
>>>>>>>    strub
>>>>>>> 
>>>>>>> 
>>>>>>>    [1]
>>>>>> 
>>> 
> http://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#relocations
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>>    ----- Original Message -----
>>>>>>>>    From: Leonardo Uribe <lu...@gmail.com>
>>>>>>>>    To: MyFaces Development 
> <de...@myfaces.apache.org>;
>>>  Mark Struberg
>>>>>>   <st...@yahoo.de>
>>>>>>>>    Cc:
>>>>>>>>    Sent: Monday, October 24, 2011 4:36 AM
>>>>>>>>    Subject: Re: [DISCUSS] how to get rid of tons of
>>>  duplicated code
>>>>>>>> 
>>>>>>>>    Hi
>>>>>>>> 
>>>>>>>>    2011/10/23 Mark Struberg 
> <st...@yahoo.de>:
>>>>>>>>>     I've now read through the old mail 
> archives and
>>>  understand
>>>>>>   what the
>>>>>>>>    original problem was. But actually I don't 
> think we
>>>  solved it
>>>>>>   correctly
>>>>>>>>    right now. Of course we solved to original 
> problem, but
>>>  opened a can of
>>>>>>   worms
>>>>>>>>    causing other problems.
>>>>>>>>> 
>>>>>>>>>     The problem as far as I remember has been 
> that
>>>  myfaces-shared had
>>>>>>   tons of
>>>>>>>>    duplicated code in it. One for core, one for 
> tomahawk, one
>>>  for
>>>>>>   trinidad, etc.
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>>     The shared part for core got moved to 
> myfaces-core,
>>>  but the deeper
>>>>>>   problem
>>>>>>>>    was that it was not easily possible to have 
> multiple
>>>  different versions
>>>>>>   of
>>>>>>>>    myfaces-shared. This now got solved by using the
>>>  maven-shade-plugin. So
>>>>>>   we
>>>>>>>>    should rethink the practice to duplicate all the 
> code and
>>>  aim for a
>>>>>>   _clean_
>>>>>>>>    solution.
>>>>>>>>> 
>>>>>>>> 
>>>>>>>>    Really that was not solved using 
> maven-shade-plugin. What
>>>  we did was
>>>>>>>>    copy the code into myfaces-core and create a 
> mirror of the
>>>  same code
>>>>>>>>    under shared. There, there is a profile called
>>>>>>>>    "synch-myfaces-impl-shared", when it is 
> added,
>>>  the code is
>>>>>>   copied and
>>>>>>>>    then a manual commit do the trick.
>>>>>>>> 
>>>>>>>>>     Also (being a maven guy) I cannot quite 
> follow the
>>>  argument about
>>>>>>   the
>>>>>>>>    release cycles. Running a myfaces-shared release 
> and then
>>>  (with the
>>>>>>   same staging
>>>>>>>>    repo) a myfaces-core release is a task of 15 
> minutes. +
>>>  the time for
>>>>>>   running the
>>>>>>>>    TCK, but this gets run via CI anyway, right? Thus 
> this is
>>>  barely a
>>>>>>   problem.
>>>>>>>>>     If it is then I'd happily volunteer to 
> do the
>>>  next release (do
>>>>>>   this for
>>>>>>>>    a few projects already) As you know, performing a 
> release
>>>  really got
>>>>>>   _much_
>>>>>>>>    easier nowadays with our new apache-parent pom.
>>>>>>>>>     But maybe this argument was only meant for 
> our old
>>>  release process
>>>>>>   (which I
>>>>>>>>    agree was a lot of work)?
>>>>>>>>> 
>>>>>>>>>     If your answer is 'it's still 
> needed'
>>>  then can we just
>>>>>>   unify
>>>>>>>>    all other usages?
>>>>>>>>> 
>>>>>>>> 
>>>>>>>>    Make a release is just the first of the problems. 
> Take
>>>  into account
>>>>>>>>    that each release requires a vote and that vote 
> takes 3
>>>  days to get
>>>>>>>>    fixed. So in practice a problem in core can 
> effectively
>>>  block a
>>>>>>>>    release of other artifacts. That's very 
> inconvenient.
>>>  Suppose we
>>>>>>   have
>>>>>>>>    a new TCK and that one found a problem on myfaces 
> core.
>>>  Again even if
>>>>>>>>    the other artifacts are good enough, this becomes 
> a
>>>  blocker. There are
>>>>>>>>    enough historical evidence that supports this 
> point. In
>>>  conclusion
>>>>>>>>    this slow down the whole release cycle we have on 
> myfaces.
>>>  So ignore
>>>>>>>>    that is not an option.
>>>>>>>> 
>>>>>>>>    Instead, maybe the option is reorganize myfaces 
> core to
>>>  allow
>>>>>>>>    alternate release lifecycles per module. For 
> example, each
>>>  maven
>>>>>>>>    plugin in myfaces has its own release lifecycle 
> and there
>>>  is a parent
>>>>>>>>    pom with a different release procedure. This 
> requires some
>>>  changes to
>>>>>>>>    create the source-release.zip file inherited from 
> apache
>>>  pom. But it
>>>>>>>>    could be a cleaner solution.
>>>>>>>> 
>>>>>>>>    This means myfaces-commons project should be
>>>  "merged" in some
>>>>>>   way with
>>>>>>>>    myfaces core. It has sense.
>>>>>>>> 
>>>>>>>>>     One question which bothers me with the
>>>  'shared' approach
>>>>>>   if what
>>>>>>>>    would happen to our build-tools annotation 
> scanning
>>>>>>   (@JSFWebConfigParam, etc)?
>>>>>>>>    Does this already work with dependencies? Do we 
> have this
>>>  problem
>>>>>>   already due to
>>>>>>>>    the fact that we import such annotated classes 
> via
>>>  dependency?
>>>>>>>>> 
>>>>>>>> 
>>>>>>>>    Those annotations comes from 
> myfaces-builder-annotations.
>>>  They are
>>>>>>>>    source code annotations but all that information 
> are saved
>>>  on
>>>>>>>>    myfaces-metadata.xml, so even if dissapear on 
> compile
>>>  time, the
>>>>>>>>    information can be gathered from there. It is not 
> a
>>>  problem.
>>>>>>>> 
>>>>>>>>>>     Additionally, we increase the risk of 
> "side
>>>>>>   effects",
>>>>>>>>>>     because a change done in core could 
> introduce a
>>>  bug in other
>>>>>>   parts.
>>>>>>>>>     Imo it's exactly the opposite. If you 
> use the
>>>  same code in 7
>>>>>>   projects,
>>>>>>>>    then it is more likely that a bug gets found and 
> fixed.
>>>>>>>>>     And the opposite case is (sadly) absolutely 
> unlikely.
>>>  If you have
>>>>>>   a class
>>>>>>>>    duplicated 7 times and find a bug in one project, 
> it is
>>>  highly unlikely
>>>>>>   that all
>>>>>>>>    6 other projects will get this fix applied :(
>>>>>>>>> 
>>>>>>>> 
>>>>>>>>    But what happen when you have some code that does 
> not have
>>>  a clear
>>>>>>>>    "interface". If somebody removes or 
> change some
>>>  code because
>>>>>>   he/she
>>>>>>>>    thinks it is not used in core or whatever, all 6 
> projects
>>>  that could
>>>>>>>>    require it will be affected and will require to 
> rework its
>>>  code.
>>>>>>>>    Things get uglier when you have one library 
> working with
>>>  version 1.1.1
>>>>>>>>    and 1.1.2 is binary incompatible with version 
> 1.1.1, but
>>>  my other
>>>>>>>>    dependency requires it and kaboooom, the 
> application does
>>>  not work.
>>>>>>>>    So, the first assumption we need to preserve in 
> those
>>>>>>   "shared"
>>>>>>>>    artifacts is build it as an API, preserving 
> binary
>>>  compatibility.
>>>>>>>> 
>>>>>>>>    So we can't just grab the code from shared as 
> is and
>>>  say to users
>>>>>>   "...
>>>>>>>>    you can use that into its own projects ...". 
> If the
>>>  project is
>>>>>>>>    maintained inside myfaces we can fix such 
> problems, but
>>>  outside
>>>>>>>>    myfaces we should be more strict. So, we need a
>>>  "public
>>>>>>   shared" code
>>>>>>>>    like the one proposed in myfaces commons and 
> other code
>>>  "myfaces
>>>>>>>>    shared" to use in projects like tomahawk or
>>>  portletbridge or
>>>>>>   whatever
>>>>>>>>    inside our land.
>>>>>>>> 
>>>>>>>>    regards,
>>>>>>>> 
>>>>>>>>    Leonardo Uribe
>>>>>>>> 
>>>>>>>>>     LieGrue,
>>>>>>>>>     strub
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>>     ----- Original Message -----
>>>>>>>>>>     From: Leonardo Uribe 
> <lu...@gmail.com>
>>>>>>>>>>     To: MyFaces Development
>>>  <de...@myfaces.apache.org>
>>>>>>>>>>     Cc: Mark Struberg 
> <st...@yahoo.de>
>>>>>>>>>>     Sent: Sunday, October 23, 2011 9:08 PM
>>>>>>>>>>     Subject: Re: [DISCUSS] how to get rid of 
> tons of
>>>  duplicated
>>>>>>   code
>>>>>>>>>> 
>>>>>>>>>>     Hi
>>>>>>>>>> 
>>>>>>>>>>     Ok, let's check the proposal
>>>>>>>>>> 
>>>>>>>>>>     MS>> So the suggestion is:
>>>>>>>>>>     MS>>
>>>>>>>>>>     MS>> 1.) cleanup myfaces-shared. 
> mf-shared
>>>  has almost no
>>>>>>>>    checkstyle
>>>>>>>>>>     rules applied.
>>>>>>>>>> 
>>>>>>>>>>     Yes, sounds good.
>>>>>>>>>> 
>>>>>>>>>>     MS>> 2.) add unit tests for 
> myfaces-shared.
>>>  Currently
>>>>>>   there are
>>>>>>>>    not
>>>>>>>>>>     many...
>>>>>>>>>> 
>>>>>>>>>>     Ok, sounds good too.
>>>>>>>>>> 
>>>>>>>>>>     MS>> 3.) move the shared code 
> parts back to
>>>>>>   myfaces-shared and
>>>>>>>>    add unit
>>>>>>>>>>     tests.
>>>>>>>>>> 
>>>>>>>>>>     So, this means do one step back and move 
> the code
>>>  from
>>>>>>   myfaces-core
>>>>>>>>>>     "shared" to myfaces-shared 
> project?
>>>  This breaks
>>>>>>   effectively
>>>>>>>>    the
>>>>>>>>>>     changes done some months ago to make 
> easier work
>>>  with myfaces
>>>>>>   core
>>>>>>>>>>     itself.
>>>>>>>>>> 
>>>>>>>>>>     In that time the conclusion was: 
> "core has
>>>  priority over
>>>>>>   anything
>>>>>>>>>>     else, so shared code must live in core, 
> but
>>>  myfaces-shared
>>>>>>   project
>>>>>>>>>>     should just copy the code from there and 
> have its
>>>  own
>>>>>>   lifecycle"
>>>>>>>>>>     (these are my own words as I 
> understood).
>>>>>>>>>> 
>>>>>>>>>>     So this point does not have practical 
> sense, and
>>>  go against
>>>>>>   everything
>>>>>>>>>>     discussed earlier.
>>>>>>>>>> 
>>>>>>>>>>     MS>> 4.) import myfaces-shared via 
> maven
>>>  dependency and
>>>>>>   use
>>>>>>>>>>     <minimizeJar> and 
> <relocations> to
>>>  package the
>>>>>>   stuff
>>>>>>>>>> 
>>>>>>>>>>     maven-shade-plugin is a good 
> "tool" but
>>>  doesn't
>>>>>>   fit well
>>>>>>>>    in this
>>>>>>>>>>     scenario. The reason is we need an 
> alternate
>>>  release lifecycle
>>>>>>   for the
>>>>>>>>>>     shared code between myfaces core and 
> other
>>>  projects.
>>>>>>>>>> 
>>>>>>>>>>     Historically that was the very first 
> intention
>>>  behind
>>>>>>   myfaces-shared
>>>>>>>>>>     project. Any myfaces core release 
> requires some
>>>  additional
>>>>>>   steps to do
>>>>>>>>>>     (TCK), so that becomes a problem when 
> you try to
>>>  release other
>>>>>>>>>>     libraries that depends of shared. So, to 
> fix
>>>  that,
>>>>>>   "shared"
>>>>>>>>    was
>>>>>>>>>>     created, so the code can be released in 
> a
>>>  independent way, and
>>>>>>   prevent
>>>>>>>>>>     myfaces core becomes an obstacle to 
> release any
>>>  other project
>>>>>>>>>>     (tomahawk, portlet-bridge, ... ). So, to 
> release
>>>  tomahawk you
>>>>>>   release
>>>>>>>>>>     shared first and then tomahawk.
>>>>>>>>>> 
>>>>>>>>>>     maven-shade-plugin requires a released 
> artifact
>>>  to do its job.
>>>>>>   So, use
>>>>>>>>>>     it impose that restriction. In 
> "shared"
>>>  case,
>>>>>>   preserve the
>>>>>>>>    original
>>>>>>>>>>     intention becomes 
> "imperative", and
>>>  that's the
>>>>>>   reason why
>>>>>>>>    a goal
>>>>>>>>>>     was
>>>>>>>>>>     created to copy the code from 
> myfaces-core
>>>  shared, so the
>>>>>>   release
>>>>>>>>>>     manager can run this goal, commit the 
> changes and
>>>  then run a
>>>>>>   release.
>>>>>>>>>> 
>>>>>>>>>>     My proposal in this case is do the same 
> we did
>>>  for shared, but
>>>>>>   for
>>>>>>>>>>     "myfaces commons" case. Then 
> we can use
>>>>>>   maven-shade-plugin in
>>>>>>>>    other
>>>>>>>>>>     projects, but not over shared, instead 
> over a
>>>  released version
>>>>>>   of
>>>>>>>>>>     myfaces-commons-utils. Keep tomahawk or
>>>  portlet-bridge as is,
>>>>>>   using
>>>>>>>>>>     shared project, because by its nature, 
> those
>>>  projects require
>>>>>>   classes
>>>>>>>>>>     that are not meant to be used outside 
> those
>>>  cases.
>>>>>>>>>> 
>>>>>>>>>>     Note do any hack in this part makes a 
> little bit
>>>>>>   "obscure"
>>>>>>>>    how to make
>>>>>>>>>>     changes, because everything becomes
>>>  "centralized",
>>>>>>   but makes
>>>>>>>>    easier
>>>>>>>>>>     maintain code. Additionally, we increase 
> the risk
>>>  of
>>>>>>   "side
>>>>>>>>    effects",
>>>>>>>>>>     because a change done in core could 
> introduce a
>>>  bug in other
>>>>>>   parts. So
>>>>>>>>>>     at the end this is a matter of how to 
> keep our
>>>  code
>>>>>>>>    "balanced", even
>>>>>>>>>>     if some times it becomes a decision 
> about
>>>  "choose the
>>>>>>   less
>>>>>>>>>>     inconvenient alternative".
>>>>>>>>>> 
>>>>>>>>>>     regards,
>>>>>>>>>> 
>>>>>>>>>>     Leonardo Uribe
>>>>>>>>>> 
>>>>>>>>>>     2011/10/23 Leonardo Uribe
>>>  <lu...@gmail.com>:
>>>>>>>>>>>      Hi
>>>>>>>>>>> 
>>>>>>>>>>>      2011/10/23 Jakob Korherr
>>>  <ja...@gmail.com>:
>>>>>>>>>>>>      Hi Mark,
>>>>>>>>>>>> 
>>>>>>>>>>>>      +1 - that's exactly what I 
> have been
>>>  trying to
>>>>>>   accomplish
>>>>>>>>    some time
>>>>>>>>>>>>      ago (introducing common-shades 
> [1]).
>>>  Unfortunately, I
>>>>>>   was not
>>>>>>>>>>>>      successful back then.
>>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>>      It is clear we need to 
> "split"
>>>  myfaces-impl
>>>>>>   into
>>>>>>>>    multiple
>>>>>>>>>>     modules. There
>>>>>>>>>>>      are some parts that are useful for 
> other
>>>  projects. The
>>>>>>   code you
>>>>>>>>    did
>>>>>>>>>>>      on commons-shade was the attempt to 
> solve
>>>  the problem of
>>>>>>   the
>>>>>>>>>>>      duplicate code used on 
> myfaces-test.
>>>>>>>>>>> 
>>>>>>>>>>>      Now the objective is find a way 
> about how to
>>>  reuse code
>>>>>>   in myfaces
>>>>>>>>>>>      core between multiple projects 
> effectively.
>>>>>>>>>>> 
>>>>>>>>>>>>      However, there is a slight 
> problem with
>>>  moving all
>>>>>>   this stuff
>>>>>>>>    into
>>>>>>>>>>>>      MyFaces shared, which I want to 
> point
>>>  out: code size.
>>>>>>   If we
>>>>>>>>    really put
>>>>>>>>>>>>      all the code that is shared 
> across any
>>>  MyFaces
>>>>>>   subproject into
>>>>>>>>    shared,
>>>>>>>>>>>>      it will get fat and ugly (even 
> more than
>>>  it is right
>>>>>>   now). In
>>>>>>>>>>>>      addition, if we continue 
> including the
>>>  whole shared
>>>>>>   project
>>>>>>>>    into
>>>>>>>>>>>>      MyFaces core, MyFaces core impl 
> will get
>>>  bigger and
>>>>>>   bigger.
>>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>>      Yes, the problem basically is 
> MyFaces shared
>>>  does not
>>>>>>   have any
>>>>>>>>    order
>>>>>>>>>>>      or any notion of API. There are 
> code that is
>>>  used only in
>>>>>>   tomahawk
>>>>>>>>    but not
>>>>>>>>>>>      intended to use in any other place. 
> There
>>>  are some useful
>>>>>>>>    utitlities but
>>>>>>>>>>>      sometimes without documentation, 
> and there
>>>  are some other
>>>>>>   code
>>>>>>>>    that is
>>>>>>>>>>>      just obsolete. It it clear a 
> cleanup of that
>>>  location is
>>>>>>>>>>>      necessary, but note priorities 
> comes first,
>>>  so this task
>>>>>>   has been
>>>>>>>>    delayed
>>>>>>>>>>     in
>>>>>>>>>>>      order to deal with other important 
> stuff.
>>>  Now it is a
>>>>>>   good time to
>>>>>>>>    fix
>>>>>>>>>>     this.
>>>>>>>>>>> 
>>>>>>>>>>>>      Thus I'd like to suggest 
> something
>>>  similar which
>>>>>>   I wanted
>>>>>>>>    to
>>>>>>>>>>>>      accomplish with common-shades: 
> Introduce
>>>  a new shared
>>>>>>   module,
>>>>>>>>    which
>>>>>>>>>>>>      consists of many submodules 
> that each
>>>  handle a
>>>>>>   specific
>>>>>>>>    functionality
>>>>>>>>>>>>      instead of being one fat 
> module. With
>>>  this approach
>>>>>>   each
>>>>>>>>    MyFaces
>>>>>>>>>>>>      subproject would be able to 
> pick out
>>>  only the stuff
>>>>>>   it really
>>>>>>>>    needs.
>>>>>>>>>>>>      Furthermore we would see more 
> easily
>>>  which code in
>>>>>>   shared is
>>>>>>>>    not used
>>>>>>>>>>>>      anymore (I guess at the moment 
> there is
>>>  a lot of it),
>>>>>>   just by
>>>>>>>>    checking
>>>>>>>>>>>>      which modules are still used in 
> our
>>>  poms.
>>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>>      That is the big question, how to 
> split
>>>  myfaces-impl and
>>>>>>   shared.
>>>>>>>>    Precisely
>>>>>>>>>>>      the intention of 
> myfaces-commons-utils
>>>  projects was take
>>>>>>   the stuff
>>>>>>>>    that is
>>>>>>>>>>>      useful from shared and build an 
> usable API
>>>  for developers
>>>>>>   outside
>>>>>>>>    MyFaces.
>>>>>>>>>>> 
>>>>>>>>>>>      For example, MyFaces HTML5 
> subproject was a
>>>  good
>>>>>>   experiment to see
>>>>>>>>>>>      which code is useful and should be 
> added in
>>>  a API. Some
>>>>>>   weeks ago
>>>>>>>>    I checked
>>>>>>>>>>>      and removed all duplicate code to 
> use
>>>>>>   myfaces-commons-utils. So
>>>>>>>>    the 1.0.2
>>>>>>>>>>>      release contains those classes 
> taken from
>>>  shared.
>>>>>>>>>>> 
>>>>>>>>>>>      regards,
>>>>>>>>>>> 
>>>>>>>>>>>      Leonardo Uribe
>>>>>>>>>>> 
>>>>>>>>>>>>      Regards,
>>>>>>>>>>>>      Jakob
>>>>>>>>>>>> 
>>>>>>>>>>>>      [1]
>>>>>>   https://svn.apache.org/repos/asf/myfaces/common-shades/
>>>>>>>>>>>> 
>>>>>>>>>>>>      2011/10/23 Mark Struberg
>>>  <st...@yahoo.de>:
>>>>>>>>>>>>>      Hi!
>>>>>>>>>>>>>      While working on the 
> mafyces-commons
>>>  cleanup I
>>>>>>   figured
>>>>>>>>    that we have
>>>>>>>>>>     tons of
>>>>>>>>>>>>>      duplicated code spread over 
> MyFaces.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>>      As an example I like to 
> mention
>>>>>>>>    myfaces-commons-resourcehandler.
>>>>>>>>>>     There are
>>>>>>>>>>>>>      43 classes in total, and 35 
> of them
>>>  are just 1:1
>>>>>>   copied
>>>>>>>>    from other
>>>>>>>>>>     projects
>>>>>>>>>>>>>      to provide resource 
> management, zip,
>>>  etc. For me
>>>>>>   this is
>>>>>>>>    an
>>>>>>>>>>     absolute no-go.
>>>>>>>>>>>>>      Those classes have neither 
> tests nor
>>>  any
>>>>>>   documentation
>>>>>>>>    where they
>>>>>>>>>>     got forked
>>>>>>>>>>>>>      from. Nor will any bug 
> which gets
>>>  fixed in
>>>>>>   another module
>>>>>>>>    make
>>>>>>>>>>     it's way over
>>>>>>>>>>>>>      to all the other projects 
> containing
>>>  that very
>>>>>>   forked
>>>>>>>>    code.
>>>>>>>>>>     That's just ...
>>>>>>>>>>>>>      unbelievable 
> unmaintainable.
>>>>>>>>>>>>> 
>>>>>>>>>>>>>      There are 2 different ways 
> to solve
>>>  this
>>>>>>   (depending on the
>>>>>>>>>>     problem):
>>>>>>>>>>>>> 
>>>>>>>>>>>>>      A.) drop the functionality 
> and
>>>  provide a
>>>>>>   generalized
>>>>>>>>    solution. The
>>>>>>>>>>     GZIP of
>>>>>>>>>>>>> 
>     myfaces-commons-resourcehandleris an
>>>  obvious
>>>>>>   example:
>>>>>>>>>>>>>      We now copy this code over 
> the 4th
>>>  time or even
>>>>>>   more.
>>>>>>>>    Instead of
>>>>>>>>>>     doing this,
>>>>>>>>>>>>>      we should rather do it in 
> the
>>>  classic unix
>>>>>>   fashion: do one
>>>>>>>>    thing,
>>>>>>>>>>     but do it
>>>>>>>>>>>>>      well.
>>>>>>>>>>>>>      Which means I'd rather 
> see all
>>>  the GZIP stuff
>>>>>>   factored
>>>>>>>>    out into
>>>>>>>>>>     an own
>>>>>>>>>>>>>      mf-commons module as a 
> Servlet
>>>  Filter. This can
>>>>>>   then get
>>>>>>>>    applied to
>>>>>>>>>>     what
>>>>>>>>>>>>>      ever other mechanism you 
> like. This
>>>  could also
>>>>>>   (commonly)
>>>>>>>>    cover
>>>>>>>>>>     cases like
>>>>>>>>>>>>>      detecting http UserAgents 
> which are
>>>  not able to
>>>>>>   handle
>>>>>>>>    zipped
>>>>>>>>>>     resources,
>>>>>>>>>>>>>      etc. That way we could 
> provide this
>>>  logic ONCE
>>>>>>   and have
>>>>>>>>    complete
>>>>>>>>>>     freedom
>>>>>>>>>>>>>      over the configuration.
>>>>>>>>>>>>> 
>>>>>>>>>>>>>      B.) code reusable 
> components once
>>>  and use them in
>>>>>>   other
>>>>>>>>    projects
>>>>>>>>>>     (ev via
>>>>>>>>>>>>>      shading it in).
>>>>>>>>>>>>>      ClassLoaderResourceLoader 
> would be a
>>>  perfect
>>>>>>   candidate! I
>>>>>>>>    grepped
>>>>>>>>>>     through
>>>>>>>>>>>>>      only the few pits which I 
> have
>>>  checked out
>>>>>>   locally and
>>>>>>>>    found this
>>>>>>>>>>     class 7
>>>>>>>>>>>>>      SEVEN times! I just 
> can't
>>>  believe that we
>>>>>>   can't
>>>>>>>>    move this
>>>>>>>>>>     stuff to a shared
>>>>>>>>>>>>>      modul...
>>>>>>>>>>>>> 
>>>>>>>>>>>>>      Same for 
> FacesServletMapping. 6
>>>  times copied
>>>>>>   around,
>>>>>>>>>>>>>      WebConfigProviderFactory 5 
> times,
>>>  ...
>>>>>>>>>>>>>      There are whole packages 
> with 10++
>>>  classes which
>>>>>>   got
>>>>>>>>    copied 1:1!
>>>>>>>>>>>>> 
>>>>>>>>>>>>>      I really could cry seeing 
> this :(
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>>      What can we do to solve 
> this?
>>>>>>>>>>>>> 
>>>>>>>>>>>>>      Theoretically 
> myfaces-shared should
>>>  contain this
>>>>>>   stuff.
>>>>>>>>    This is
>>>>>>>>>>     exactly what
>>>>>>>>>>>>>      it is for!
>>>>>>>>>>>>>      Historically there have 
> been some
>>>  hand forged
>>>>>>   tweeks and
>>>>>>>>    ugly
>>>>>>>>>>     hacks, but
>>>>>>>>>>>>>      nowadays we have the
>>>  maven-shade-plugin to make
>>>>>>   our live
>>>>>>>>    easier.
>>>>>>>>>>>>> 
>>>>>>>>>>>>>      So the suggestion is:
>>>>>>>>>>>>> 
>>>>>>>>>>>>>      1.) cleanup myfaces-shared.
>>>  mf-shared has almost
>>>>>>   no
>>>>>>>>    checkstyle
>>>>>>>>>>     rules
>>>>>>>>>>>>>      applied.
>>>>>>>>>>>>>      2.) add unit tests for
>>>  myfaces-shared. Currently
>>>>>>   there are
>>>>>>>>    not
>>>>>>>>>>     many...
>>>>>>>>>>>>>      3.) move the shared code 
> parts back
>>>  to
>>>>>>   myfaces-shared and
>>>>>>>>    add unit
>>>>>>>>>>     tests.
>>>>>>>>>>>>>      4.) import myfaces-shared 
> via maven
>>>  dependency
>>>>>>   and use
>>>>>>>>>>     <minimizeJar> and
>>>>>>>>>>>>>      <relocations> to 
> package the
>>>  stuff
>>>>>>>>>>>>> 
>>>>>>>>>>>>>      [+1] fine go ahead 
> (ideally: yes,
>>>  what parts can
>>>>>>   I help
>>>>>>>>    with?)
>>>>>>>>>>>>>      [0] dont care
>>>>>>>>>>>>>      [-1] wont work because ...
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>>      I've attached a file 
> which
>>>  contains all
>>>>>>   Classes which
>>>>>>>>    name
>>>>>>>>>>     exists multiple
>>>>>>>>>>>>>      times in MyFaces. The 
> number is the
>>>  cound how
>>>>>>   often they
>>>>>>>>    exist in
>>>>>>>>>>     MyFaces. I
>>>>>>>>>>>>>      excluded current20.
>>>>>>>>>>>>>      Please note that classes 
> with the
>>>  same name do
>>>>>>   not
>>>>>>>>    necessarily have
>>>>>>>>>>     the same
>>>>>>>>>>>>>      content - but quite a lot 
> actually
>>>  do have!
>>>>>>   (scroll to the
>>>>>>>>    bottom
>>>>>>>>>>     of the
>>>>>>>>>>>>>      file ...)
>>>>>>>>>>>>> 
>>>>>>>>>>>>>      LieGrue,
>>>>>>>>>>>>>      strub
>>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>>      --
>>>>>>>>>>>>      Jakob Korherr
>>>>>>>>>>>> 
>>>>>>>>>>>>      blog: http://www.jakobk.com
>>>>>>>>>>>>      twitter: 
> http://twitter.com/jakobkorherr
>>>>>>>>>>>>      work: http://www.irian.at
>>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>

Re: [DISCUSS] how to get rid of tons of duplicated code

[Leonardo Uribe <lu...@gmail.com>]

2011/10/25 Mark Struberg <st...@yahoo.de>:
> Oki, just found time to do a  review:
>
> 1.) why do we need an own 'parent' module? I think all the basic plugin definitions should stay the same for all myfaces-core project, don't they?
>

To allow release shared-utils or shared module without the burden to
release all myfaces core. For example, if the last release was 2.1.4,
we can release shared internal module with version number 2.1.4.1. The
pom on 'parent' module has all base definitions that should stay the
same for all myfaces-core project, so things have not changed, just
think about this as a simple file relocation, as simple as that.

> 2.)
>> shared-utils : utilities for JSF used in core, but built as an api.
> what do you mean with 'built as an api' ?
>

It means code there should documented, tested, should preserve binary
compatibility against backward versions on the same artifact, and
should be useful outside myfaces projects. In other words, code there
has a clear "interface". 'shared' module continue working as usual,
just a place to share code with tomahawk and other internal myfaces
projects. The idea is if we create some utility on shared and we found
it is good enough later we move it to shared-utils, and at the end do
a cleanup over all shared code.

>
> 3.)
>> Now, we move the duplicate code related to myfaces-commons-utils
>
>> into this module
> +1 to dropping myfaces-commons-utils. This is just a 1:1 fork from shared.
> Could you please explain again where you like to move/copy this parts to now?
>

I already sync the code available in myfaces-commons-utils with the
duplicate code in shared, using a diff tool. The next step is use
maven-shade-plugin to grab the code of the shared-utils internal
module and do a package rename, removing the duplicate code from
myfaces-commons-utils. In practice, it should only have some resource
files related to i18n, but no java files (because those files will be
maintained from shared-utils).

> 4.)
>> and shared will have shared-utils as dependency
> Which 'shared-utils' do you mean? the newly proposed module myfaces/core/shared ?
> Actually myfaces/shared is then not needed anymore, right? This would be a pretty big change to what has been.
>
> To be honest: I'd rather make the unique source of all those things in myfaces/shared which it has been for years until only recently as this only got changed in July or August this year! I really just see no benefit in keeping this at myfaces/core/ over having it well maintained in myfaces/shared.
> If we do that, we also don't need any ugly hacks while doing async releases - that's exactly what such a library is designed for
>

Look that shared uses maven-shade-plugin to include shared-utils into
its jar. Tomahawk and other projects still needs shared module as is.
But those projects can use myfaces-core shared internal module
directly, so myfaces/shared is no longer necessary.

The benefit of keep things at myfaces/core is it simplify testing.
Before if a bug in shared was found, it was required to build shared,
core and the test webapp. With these change you only need to build
core and the test webapp. This was the major complain about shared,
and the reason why most people wanted to remove it.

The release procedure is not a problem with the new strategy proposed,
because this one has been widely tested before and really don't
enforce any hack at all. All magic was done in the pom relocation to
parent module and other small changes.

> 5.)
>> Then we kill shared-tomahawk, and we make tomahawk use
>> maven-shade-plugin ..
>
>  +1
> Actually this will be the biggest amount of work. To get rid of and unify all the copied sources again.
> But I really think this is way worth it!
>
>
> Summary:
>
> I think we now agree that we should aim to 'unify' our code which got copied multiple times and pull them back to a common usable library, right?
>
> We are not yet d'accord if this common library should reside in myfaces/core/shared (as part of MyFaces core) or myfaces/shared (as a separate MyFaces subproject). What about listing pros and cons and then start a vote?
>

As is considered, myfaces/core/shared and myfaces/core/shared-utils
are internal modules. myfaces-commons-utils library just take
shared-utils, repackage and makes it available to public. At the end,
the end user will never see the difference. In practice, with this
proposal we are only putting things in order, but we are not changing
the 'substance'.

regards,

Leonardo U

>
> txs and LieGrue,
> strub
>
>
> PS: no worry, together we will soon solve this riddle ;)
>
>
>
> ----- Original Message -----
>> From: Leonardo Uribe <lu...@gmail.com>
>> To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg <st...@yahoo.de>
>> Cc:
>> Sent: Tuesday, October 25, 2011 4:02 AM
>> Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>
>> Hi
>>
>> Ok, it is done. In the branch it is possible to see how core will look
>> after the change. The other changes are just use maven-shade plugin
>> over myfaces-impl-shared-utils and myfaces-impl-shared internal
>> modules, and do some package relocation in some cases.
>>
>> regards
>>
>> Leonardo Uribe
>>
>> 2011/10/24 Leonardo Uribe <lu...@gmail.com>:
>>>  Hi
>>>
>>>  I started a branch to try it:
>>>
>>>
>> http://svn.apache.org/repos/asf/myfaces/core/branches/2.0.x_refactor_shade_duplicate/
>>>
>>>  just to gain some time. I think it is worth at least to try a prototype.
>>>
>>>  regards,
>>>
>>>  Leonardo Uribe
>>>
>>>  2011/10/24 Mark Struberg <st...@yahoo.de>:
>>>>  Hi Leo!
>>>>
>>>>  Yes, this sounds much better, but please give me 2 days to think
>> through it in detail.
>>>>
>>>>  txs and LieGrue,
>>>>  strub
>>>>
>>>>
>>>>
>>>>  ----- Original Message -----
>>>>>  From: Leonardo Uribe <lu...@gmail.com>
>>>>>  To: MyFaces Development <de...@myfaces.apache.org>; Mark
>> Struberg <st...@yahoo.de>
>>>>>  Cc:
>>>>>  Sent: Monday, October 24, 2011 7:18 PM
>>>>>  Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>>>>
>>>>>  Hi
>>>>>
>>>>>  Ok, it is evident we have different opinions here about how to deal
>>>>>  with this problem. So, rather than refute the arguments I think it
>> is
>>>>>  more productive to show another proposal. In MyFaces Core 2.0.x we
>>>>>  have the following layout:
>>>>>
>>>>>  api/
>>>>>  assembly/
>>>>>  bundle/
>>>>>  impl/
>>>>>  implee6/
>>>>>  integration-tests/
>>>>>  pom.xml
>>>>>  shared/
>>>>>  src/
>>>>>
>>>>>  Let's add two modules
>>>>>
>>>>>  parent : contains the parent POM that all submodules should
>> inherit.
>>>>>  shared-utils : utilities for JSF used in core, but built as an api.
>>>>>
>>>>>  Now, we move the duplicate code related to myfaces-commons-utils
>> into
>>>>>  this module, and shared will have shared-utils as dependency. impl
>>>>>  module will use maven-shade plugin to take the code from
>> shared-utils
>>>>>  and shared (actually this is done for only for shared).
>>>>>
>>>>>  When a release of myfaces-core is done, all modules are released,
>> so
>>>>>  things go as usual. But have the parent as module has the advantage
>>>>>  that if we want to release shared-utils or shared internal modules,
>> we
>>>>>  can do it only releasing parent (optional) and shared-utils.
>>>>>
>>>>>  Since we can create a release for these modules, we remove the hack
>>>>>  used on on shared project (hard copy). Just we modify the pom to
>> use
>>>>>  maven-shade-plugin over myfaces-core shared module. Then we kill
>>>>>  shared-tomahawk, and we make tomahawk use maven-shade-plugin over
>>>>>  shaded artifact in shared project.
>>>>>
>>>>>  In commons we do the same as in shared. In
>>>>>  myfaces-commons-resourcehandler we use myfaces core shared module
>>>>>  using maven-shade-plugin.
>>>>>
>>>>>  The only disadvantage is the release process gets a little bit more
>>>>>  complicated, but since do a release becomes easier with nexus, it
>> is
>>>>>  ok.
>>>>>
>>>>>  Do you agree with this solution?
>>>>>
>>>>>  regards,
>>>>>
>>>>>  Leonardo Uribe
>>>>>
>>>>>  2011/10/24 Mark Struberg <st...@yahoo.de>:
>>>>>>>  Really that was not solved using maven-shade-plugin.
>>>>>>   Then we used the maven-shade-plugin the wrong way. See the
>>>>>  <relocations> option [1].
>>>>>>
>>>>>>>   There, there is a profile called
>>>>>>>   "synch-myfaces-impl-shared", when it is added,
>> the code is
>>>>>  copied and
>>>>>>>   then a manual commit do the trick.
>>>>>>
>>>>>>   I think this is an ugly hack and doesn't solve any
>> problems because
>>>>>>
>>>>>>   a.)
>>>>>>>   Take into account
>>>>>>>   that each release requires a vote and that vote takes 3
>> days to get
>>>>>>>   fixed.
>>>>>>   you could just do a mvn release of shared + core in 1 go to
>> the same
>>>>>  staging repo -> only 1 vote is needed!
>>>>>>   This argument is simply wrong.
>>>>>>
>>>>>>   b.)
>>>>>>   You
>>>>>>    currently copy the code over 1:1 (half manually) thus your
>> argument
>>>>>>   with 'core and other projects need different sources'
>> is just nil.
>>>>>  There
>>>>>>    is no difference if you do this by profile, by hand or
>> automatically!
>>>>>>   So I really prefer to have this automatically. Which is
>> exactly what a
>>>>>>   dependency does...
>>>>>>
>>>>>>   c.) the TCK argument is moot because it has
>>>>>>   nothing to do with shared. Most of the issues in the TCK are
>> not
>>>>>>   affecting shared. And if they do, then those fixes are needed
>> BY ALL
>>>>>>   OTHER PROJECTS AS WELL. Thus another argument against hiding
>> this code
>>>>>>   and duplicating it all over...
>>>>>>
>>>>>>   c.)
>>>>>>>   Instead, maybe the option is reorganize myfaces core to
>> allow
>>>>>>>   alternate release lifecycles per module
>>>>>>   Sorry I don't grok this argument. It sounds as it would
>> make all things
>>>>>  more complicated without solving any real problem.
>>>>>>
>>>>>>   e.)
>>>>>>>   This means myfaces-commons project should be
>> "merged" in some
>>>>>  way with
>>>>>>>   myfaces core. It has sense.
>>>>>>   2 options:
>>>>>>   1..) kill myfaces-shared completely and use the shared from
>> core instead.
>>>>>>   Downside: if you need some fix in the shared code for some
>> other project,
>>>>>  you would need to release mf-core
>>>>>>   2.) kill the newly introduced (this got only created a few
>> weeks back by
>>>>>  you) core-shared and use mf-shared again.
>>>>>>   Downside: hmmm nothing if one understands how to release
>> correctly.
>>>>>>
>>>>>>   f.)
>>>>>>    all your explanations only explain the duplication between
>>>>>>   myfaces-shared and myfaces-core-shared. I can live with the
>> duplication
>>>>>>   for now, but we also have classes which got copied around up
>> to 8 times!
>>>>>>    There is no excuse for that imo.
>>>>>>
>>>>>>   g.)
>>>>>>>   But what happen when you have some code that does not have
>> a clear
>>>>>>>   "interface". If somebody removes or change some
>> code because
>>>>>  he/she
>>>>>>>   thinks it is not used in core or whatever, all 6 projects
>> that could
>>>>>>>   require it will be affected and will require to rework its
>> code.
>>>>>>>   Things get uglier when you have one library working with
>> version 1.1.1
>>>>>>>   and 1.1.2 is binary incompatible with version 1.1.1, but
>> my other
>>>>>>>   dependency requires it and kaboooom, the application does
>> not work.
>>>>>>>   So, the first assumption we need to preserve in those
>>>>>  "shared"
>>>>>>>   artifacts is build it as an API, preserving binary
>> compatibility.
>>>>>>
>>>>>>   I don't get that argument neither!
>>>>>>   Hey,
>>>>>>    that's life! If it turns out that the code is not good
>> enough and
>>>>>  needs
>>>>>>    a fix, then that's the way it is! All other projects
>> should fix that
>>>>>>   too in that case. I rather have a reproducible compile error
>> which
>>>>>>   easily could get fixed than having tons of duplicated code
>> which is more
>>>>>>    or less always logically broken and badly tested.
>>>>>>   Yes, we should be
>>>>>>   aware that the classes we put into myfaces-shared must meet
>> some
>>>>>>   standards and need to be well tested. But actually that would
>> benefit
>>>>>>   our project a lot.
>>>>>>
>>>>>>
>>>>>>   h.) I just realised that our process in copying shared-impl
>> from core to
>>>>>  mf-shared is even more broken than every process before.
>>>>>>   If you are working on a lets say mf-commons project and find a
>> bug in any
>>>>>  of those shared parts, then you would need to RELEASE MF-CORE
>> FIRST? omg, this
>>>>>  cannot be serious!
>>>>>>
>>>>>>
>>>>>>   LieGrue,
>>>>>>   strub
>>>>>>
>>>>>>
>>>>>>   [1]
>>>>>
>> http://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#relocations
>>>>>>
>>>>>>
>>>>>>
>>>>>>   ----- Original Message -----
>>>>>>>   From: Leonardo Uribe <lu...@gmail.com>
>>>>>>>   To: MyFaces Development <de...@myfaces.apache.org>;
>> Mark Struberg
>>>>>  <st...@yahoo.de>
>>>>>>>   Cc:
>>>>>>>   Sent: Monday, October 24, 2011 4:36 AM
>>>>>>>   Subject: Re: [DISCUSS] how to get rid of tons of
>> duplicated code
>>>>>>>
>>>>>>>   Hi
>>>>>>>
>>>>>>>   2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>>>>>>    I've now read through the old mail archives and
>> understand
>>>>>  what the
>>>>>>>   original problem was. But actually I don't think we
>> solved it
>>>>>  correctly
>>>>>>>   right now. Of course we solved to original problem, but
>> opened a can of
>>>>>  worms
>>>>>>>   causing other problems.
>>>>>>>>
>>>>>>>>    The problem as far as I remember has been that
>> myfaces-shared had
>>>>>  tons of
>>>>>>>   duplicated code in it. One for core, one for tomahawk, one
>> for
>>>>>  trinidad, etc.
>>>>>>>>
>>>>>>>>
>>>>>>>>    The shared part for core got moved to myfaces-core,
>> but the deeper
>>>>>  problem
>>>>>>>   was that it was not easily possible to have multiple
>> different versions
>>>>>  of
>>>>>>>   myfaces-shared. This now got solved by using the
>> maven-shade-plugin. So
>>>>>  we
>>>>>>>   should rethink the practice to duplicate all the code and
>> aim for a
>>>>>  _clean_
>>>>>>>   solution.
>>>>>>>>
>>>>>>>
>>>>>>>   Really that was not solved using maven-shade-plugin. What
>> we did was
>>>>>>>   copy the code into myfaces-core and create a mirror of the
>> same code
>>>>>>>   under shared. There, there is a profile called
>>>>>>>   "synch-myfaces-impl-shared", when it is added,
>> the code is
>>>>>  copied and
>>>>>>>   then a manual commit do the trick.
>>>>>>>
>>>>>>>>    Also (being a maven guy) I cannot quite follow the
>> argument about
>>>>>  the
>>>>>>>   release cycles. Running a myfaces-shared release and then
>> (with the
>>>>>  same staging
>>>>>>>   repo) a myfaces-core release is a task of 15 minutes. +
>> the time for
>>>>>  running the
>>>>>>>   TCK, but this gets run via CI anyway, right? Thus this is
>> barely a
>>>>>  problem.
>>>>>>>>    If it is then I'd happily volunteer to do the
>> next release (do
>>>>>  this for
>>>>>>>   a few projects already) As you know, performing a release
>> really got
>>>>>  _much_
>>>>>>>   easier nowadays with our new apache-parent pom.
>>>>>>>>    But maybe this argument was only meant for our old
>> release process
>>>>>  (which I
>>>>>>>   agree was a lot of work)?
>>>>>>>>
>>>>>>>>    If your answer is 'it's still needed'
>> then can we just
>>>>>  unify
>>>>>>>   all other usages?
>>>>>>>>
>>>>>>>
>>>>>>>   Make a release is just the first of the problems. Take
>> into account
>>>>>>>   that each release requires a vote and that vote takes 3
>> days to get
>>>>>>>   fixed. So in practice a problem in core can effectively
>> block a
>>>>>>>   release of other artifacts. That's very inconvenient.
>> Suppose we
>>>>>  have
>>>>>>>   a new TCK and that one found a problem on myfaces core.
>> Again even if
>>>>>>>   the other artifacts are good enough, this becomes a
>> blocker. There are
>>>>>>>   enough historical evidence that supports this point. In
>> conclusion
>>>>>>>   this slow down the whole release cycle we have on myfaces.
>> So ignore
>>>>>>>   that is not an option.
>>>>>>>
>>>>>>>   Instead, maybe the option is reorganize myfaces core to
>> allow
>>>>>>>   alternate release lifecycles per module. For example, each
>> maven
>>>>>>>   plugin in myfaces has its own release lifecycle and there
>> is a parent
>>>>>>>   pom with a different release procedure. This requires some
>> changes to
>>>>>>>   create the source-release.zip file inherited from apache
>> pom. But it
>>>>>>>   could be a cleaner solution.
>>>>>>>
>>>>>>>   This means myfaces-commons project should be
>> "merged" in some
>>>>>  way with
>>>>>>>   myfaces core. It has sense.
>>>>>>>
>>>>>>>>    One question which bothers me with the
>> 'shared' approach
>>>>>  if what
>>>>>>>   would happen to our build-tools annotation scanning
>>>>>  (@JSFWebConfigParam, etc)?
>>>>>>>   Does this already work with dependencies? Do we have this
>> problem
>>>>>  already due to
>>>>>>>   the fact that we import such annotated classes via
>> dependency?
>>>>>>>>
>>>>>>>
>>>>>>>   Those annotations comes from myfaces-builder-annotations.
>> They are
>>>>>>>   source code annotations but all that information are saved
>> on
>>>>>>>   myfaces-metadata.xml, so even if dissapear on compile
>> time, the
>>>>>>>   information can be gathered from there. It is not a
>> problem.
>>>>>>>
>>>>>>>>>    Additionally, we increase the risk of "side
>>>>>  effects",
>>>>>>>>>    because a change done in core could introduce a
>> bug in other
>>>>>  parts.
>>>>>>>>    Imo it's exactly the opposite. If you use the
>> same code in 7
>>>>>  projects,
>>>>>>>   then it is more likely that a bug gets found and fixed.
>>>>>>>>    And the opposite case is (sadly) absolutely unlikely.
>> If you have
>>>>>  a class
>>>>>>>   duplicated 7 times and find a bug in one project, it is
>> highly unlikely
>>>>>  that all
>>>>>>>   6 other projects will get this fix applied :(
>>>>>>>>
>>>>>>>
>>>>>>>   But what happen when you have some code that does not have
>> a clear
>>>>>>>   "interface". If somebody removes or change some
>> code because
>>>>>  he/she
>>>>>>>   thinks it is not used in core or whatever, all 6 projects
>> that could
>>>>>>>   require it will be affected and will require to rework its
>> code.
>>>>>>>   Things get uglier when you have one library working with
>> version 1.1.1
>>>>>>>   and 1.1.2 is binary incompatible with version 1.1.1, but
>> my other
>>>>>>>   dependency requires it and kaboooom, the application does
>> not work.
>>>>>>>   So, the first assumption we need to preserve in those
>>>>>  "shared"
>>>>>>>   artifacts is build it as an API, preserving binary
>> compatibility.
>>>>>>>
>>>>>>>   So we can't just grab the code from shared as is and
>> say to users
>>>>>  "...
>>>>>>>   you can use that into its own projects ...". If the
>> project is
>>>>>>>   maintained inside myfaces we can fix such problems, but
>> outside
>>>>>>>   myfaces we should be more strict. So, we need a
>> "public
>>>>>  shared" code
>>>>>>>   like the one proposed in myfaces commons and other code
>> "myfaces
>>>>>>>   shared" to use in projects like tomahawk or
>> portletbridge or
>>>>>  whatever
>>>>>>>   inside our land.
>>>>>>>
>>>>>>>   regards,
>>>>>>>
>>>>>>>   Leonardo Uribe
>>>>>>>
>>>>>>>>    LieGrue,
>>>>>>>>    strub
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>    ----- Original Message -----
>>>>>>>>>    From: Leonardo Uribe <lu...@gmail.com>
>>>>>>>>>    To: MyFaces Development
>> <de...@myfaces.apache.org>
>>>>>>>>>    Cc: Mark Struberg <st...@yahoo.de>
>>>>>>>>>    Sent: Sunday, October 23, 2011 9:08 PM
>>>>>>>>>    Subject: Re: [DISCUSS] how to get rid of tons of
>> duplicated
>>>>>  code
>>>>>>>>>
>>>>>>>>>    Hi
>>>>>>>>>
>>>>>>>>>    Ok, let's check the proposal
>>>>>>>>>
>>>>>>>>>    MS>> So the suggestion is:
>>>>>>>>>    MS>>
>>>>>>>>>    MS>> 1.) cleanup myfaces-shared. mf-shared
>> has almost no
>>>>>>>   checkstyle
>>>>>>>>>    rules applied.
>>>>>>>>>
>>>>>>>>>    Yes, sounds good.
>>>>>>>>>
>>>>>>>>>    MS>> 2.) add unit tests for myfaces-shared.
>> Currently
>>>>>  there are
>>>>>>>   not
>>>>>>>>>    many...
>>>>>>>>>
>>>>>>>>>    Ok, sounds good too.
>>>>>>>>>
>>>>>>>>>    MS>> 3.) move the shared code parts back to
>>>>>  myfaces-shared and
>>>>>>>   add unit
>>>>>>>>>    tests.
>>>>>>>>>
>>>>>>>>>    So, this means do one step back and move the code
>> from
>>>>>  myfaces-core
>>>>>>>>>    "shared" to myfaces-shared project?
>> This breaks
>>>>>  effectively
>>>>>>>   the
>>>>>>>>>    changes done some months ago to make easier work
>> with myfaces
>>>>>  core
>>>>>>>>>    itself.
>>>>>>>>>
>>>>>>>>>    In that time the conclusion was: "core has
>> priority over
>>>>>  anything
>>>>>>>>>    else, so shared code must live in core, but
>> myfaces-shared
>>>>>  project
>>>>>>>>>    should just copy the code from there and have its
>> own
>>>>>  lifecycle"
>>>>>>>>>    (these are my own words as I understood).
>>>>>>>>>
>>>>>>>>>    So this point does not have practical sense, and
>> go against
>>>>>  everything
>>>>>>>>>    discussed earlier.
>>>>>>>>>
>>>>>>>>>    MS>> 4.) import myfaces-shared via maven
>> dependency and
>>>>>  use
>>>>>>>>>    <minimizeJar> and <relocations> to
>> package the
>>>>>  stuff
>>>>>>>>>
>>>>>>>>>    maven-shade-plugin is a good "tool" but
>> doesn't
>>>>>  fit well
>>>>>>>   in this
>>>>>>>>>    scenario. The reason is we need an alternate
>> release lifecycle
>>>>>  for the
>>>>>>>>>    shared code between myfaces core and other
>> projects.
>>>>>>>>>
>>>>>>>>>    Historically that was the very first intention
>> behind
>>>>>  myfaces-shared
>>>>>>>>>    project. Any myfaces core release requires some
>> additional
>>>>>  steps to do
>>>>>>>>>    (TCK), so that becomes a problem when you try to
>> release other
>>>>>>>>>    libraries that depends of shared. So, to fix
>> that,
>>>>>  "shared"
>>>>>>>   was
>>>>>>>>>    created, so the code can be released in a
>> independent way, and
>>>>>  prevent
>>>>>>>>>    myfaces core becomes an obstacle to release any
>> other project
>>>>>>>>>    (tomahawk, portlet-bridge, ... ). So, to release
>> tomahawk you
>>>>>  release
>>>>>>>>>    shared first and then tomahawk.
>>>>>>>>>
>>>>>>>>>    maven-shade-plugin requires a released artifact
>> to do its job.
>>>>>  So, use
>>>>>>>>>    it impose that restriction. In "shared"
>> case,
>>>>>  preserve the
>>>>>>>   original
>>>>>>>>>    intention becomes "imperative", and
>> that's the
>>>>>  reason why
>>>>>>>   a goal
>>>>>>>>>    was
>>>>>>>>>    created to copy the code from myfaces-core
>> shared, so the
>>>>>  release
>>>>>>>>>    manager can run this goal, commit the changes and
>> then run a
>>>>>  release.
>>>>>>>>>
>>>>>>>>>    My proposal in this case is do the same we did
>> for shared, but
>>>>>  for
>>>>>>>>>    "myfaces commons" case. Then we can use
>>>>>  maven-shade-plugin in
>>>>>>>   other
>>>>>>>>>    projects, but not over shared, instead over a
>> released version
>>>>>  of
>>>>>>>>>    myfaces-commons-utils. Keep tomahawk or
>> portlet-bridge as is,
>>>>>  using
>>>>>>>>>    shared project, because by its nature, those
>> projects require
>>>>>  classes
>>>>>>>>>    that are not meant to be used outside those
>> cases.
>>>>>>>>>
>>>>>>>>>    Note do any hack in this part makes a little bit
>>>>>  "obscure"
>>>>>>>   how to make
>>>>>>>>>    changes, because everything becomes
>> "centralized",
>>>>>  but makes
>>>>>>>   easier
>>>>>>>>>    maintain code. Additionally, we increase the risk
>> of
>>>>>  "side
>>>>>>>   effects",
>>>>>>>>>    because a change done in core could introduce a
>> bug in other
>>>>>  parts. So
>>>>>>>>>    at the end this is a matter of how to keep our
>> code
>>>>>>>   "balanced", even
>>>>>>>>>    if some times it becomes a decision about
>> "choose the
>>>>>  less
>>>>>>>>>    inconvenient alternative".
>>>>>>>>>
>>>>>>>>>    regards,
>>>>>>>>>
>>>>>>>>>    Leonardo Uribe
>>>>>>>>>
>>>>>>>>>    2011/10/23 Leonardo Uribe
>> <lu...@gmail.com>:
>>>>>>>>>>     Hi
>>>>>>>>>>
>>>>>>>>>>     2011/10/23 Jakob Korherr
>> <ja...@gmail.com>:
>>>>>>>>>>>     Hi Mark,
>>>>>>>>>>>
>>>>>>>>>>>     +1 - that's exactly what I have been
>> trying to
>>>>>  accomplish
>>>>>>>   some time
>>>>>>>>>>>     ago (introducing common-shades [1]).
>> Unfortunately, I
>>>>>  was not
>>>>>>>>>>>     successful back then.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>     It is clear we need to "split"
>> myfaces-impl
>>>>>  into
>>>>>>>   multiple
>>>>>>>>>    modules. There
>>>>>>>>>>     are some parts that are useful for other
>> projects. The
>>>>>  code you
>>>>>>>   did
>>>>>>>>>>     on commons-shade was the attempt to solve
>> the problem of
>>>>>  the
>>>>>>>>>>     duplicate code used on myfaces-test.
>>>>>>>>>>
>>>>>>>>>>     Now the objective is find a way about how to
>> reuse code
>>>>>  in myfaces
>>>>>>>>>>     core between multiple projects effectively.
>>>>>>>>>>
>>>>>>>>>>>     However, there is a slight problem with
>> moving all
>>>>>  this stuff
>>>>>>>   into
>>>>>>>>>>>     MyFaces shared, which I want to point
>> out: code size.
>>>>>  If we
>>>>>>>   really put
>>>>>>>>>>>     all the code that is shared across any
>> MyFaces
>>>>>  subproject into
>>>>>>>   shared,
>>>>>>>>>>>     it will get fat and ugly (even more than
>> it is right
>>>>>  now). In
>>>>>>>>>>>     addition, if we continue including the
>> whole shared
>>>>>  project
>>>>>>>   into
>>>>>>>>>>>     MyFaces core, MyFaces core impl will get
>> bigger and
>>>>>  bigger.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>     Yes, the problem basically is MyFaces shared
>> does not
>>>>>  have any
>>>>>>>   order
>>>>>>>>>>     or any notion of API. There are code that is
>> used only in
>>>>>  tomahawk
>>>>>>>   but not
>>>>>>>>>>     intended to use in any other place. There
>> are some useful
>>>>>>>   utitlities but
>>>>>>>>>>     sometimes without documentation, and there
>> are some other
>>>>>  code
>>>>>>>   that is
>>>>>>>>>>     just obsolete. It it clear a cleanup of that
>> location is
>>>>>>>>>>     necessary, but note priorities comes first,
>> so this task
>>>>>  has been
>>>>>>>   delayed
>>>>>>>>>    in
>>>>>>>>>>     order to deal with other important stuff.
>> Now it is a
>>>>>  good time to
>>>>>>>   fix
>>>>>>>>>    this.
>>>>>>>>>>
>>>>>>>>>>>     Thus I'd like to suggest something
>> similar which
>>>>>  I wanted
>>>>>>>   to
>>>>>>>>>>>     accomplish with common-shades: Introduce
>> a new shared
>>>>>  module,
>>>>>>>   which
>>>>>>>>>>>     consists of many submodules that each
>> handle a
>>>>>  specific
>>>>>>>   functionality
>>>>>>>>>>>     instead of being one fat module. With
>> this approach
>>>>>  each
>>>>>>>   MyFaces
>>>>>>>>>>>     subproject would be able to pick out
>> only the stuff
>>>>>  it really
>>>>>>>   needs.
>>>>>>>>>>>     Furthermore we would see more easily
>> which code in
>>>>>  shared is
>>>>>>>   not used
>>>>>>>>>>>     anymore (I guess at the moment there is
>> a lot of it),
>>>>>  just by
>>>>>>>   checking
>>>>>>>>>>>     which modules are still used in our
>> poms.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>     That is the big question, how to split
>> myfaces-impl and
>>>>>  shared.
>>>>>>>   Precisely
>>>>>>>>>>     the intention of myfaces-commons-utils
>> projects was take
>>>>>  the stuff
>>>>>>>   that is
>>>>>>>>>>     useful from shared and build an usable API
>> for developers
>>>>>  outside
>>>>>>>   MyFaces.
>>>>>>>>>>
>>>>>>>>>>     For example, MyFaces HTML5 subproject was a
>> good
>>>>>  experiment to see
>>>>>>>>>>     which code is useful and should be added in
>> a API. Some
>>>>>  weeks ago
>>>>>>>   I checked
>>>>>>>>>>     and removed all duplicate code to use
>>>>>  myfaces-commons-utils. So
>>>>>>>   the 1.0.2
>>>>>>>>>>     release contains those classes taken from
>> shared.
>>>>>>>>>>
>>>>>>>>>>     regards,
>>>>>>>>>>
>>>>>>>>>>     Leonardo Uribe
>>>>>>>>>>
>>>>>>>>>>>     Regards,
>>>>>>>>>>>     Jakob
>>>>>>>>>>>
>>>>>>>>>>>     [1]
>>>>>  https://svn.apache.org/repos/asf/myfaces/common-shades/
>>>>>>>>>>>
>>>>>>>>>>>     2011/10/23 Mark Struberg
>> <st...@yahoo.de>:
>>>>>>>>>>>>     Hi!
>>>>>>>>>>>>     While working on the mafyces-commons
>> cleanup I
>>>>>  figured
>>>>>>>   that we have
>>>>>>>>>    tons of
>>>>>>>>>>>>     duplicated code spread over MyFaces.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>     As an example I like to mention
>>>>>>>   myfaces-commons-resourcehandler.
>>>>>>>>>    There are
>>>>>>>>>>>>     43 classes in total, and 35 of them
>> are just 1:1
>>>>>  copied
>>>>>>>   from other
>>>>>>>>>    projects
>>>>>>>>>>>>     to provide resource management, zip,
>> etc. For me
>>>>>  this is
>>>>>>>   an
>>>>>>>>>    absolute no-go.
>>>>>>>>>>>>     Those classes have neither tests nor
>> any
>>>>>  documentation
>>>>>>>   where they
>>>>>>>>>    got forked
>>>>>>>>>>>>     from. Nor will any bug which gets
>> fixed in
>>>>>  another module
>>>>>>>   make
>>>>>>>>>    it's way over
>>>>>>>>>>>>     to all the other projects containing
>> that very
>>>>>  forked
>>>>>>>   code.
>>>>>>>>>    That's just ...
>>>>>>>>>>>>     unbelievable unmaintainable.
>>>>>>>>>>>>
>>>>>>>>>>>>     There are 2 different ways to solve
>> this
>>>>>  (depending on the
>>>>>>>>>    problem):
>>>>>>>>>>>>
>>>>>>>>>>>>     A.) drop the functionality and
>> provide a
>>>>>  generalized
>>>>>>>   solution. The
>>>>>>>>>    GZIP of
>>>>>>>>>>>>     myfaces-commons-resourcehandleris an
>> obvious
>>>>>  example:
>>>>>>>>>>>>     We now copy this code over the 4th
>> time or even
>>>>>  more.
>>>>>>>   Instead of
>>>>>>>>>    doing this,
>>>>>>>>>>>>     we should rather do it in the
>> classic unix
>>>>>  fashion: do one
>>>>>>>   thing,
>>>>>>>>>    but do it
>>>>>>>>>>>>     well.
>>>>>>>>>>>>     Which means I'd rather see all
>> the GZIP stuff
>>>>>  factored
>>>>>>>   out into
>>>>>>>>>    an own
>>>>>>>>>>>>     mf-commons module as a Servlet
>> Filter. This can
>>>>>  then get
>>>>>>>   applied to
>>>>>>>>>    what
>>>>>>>>>>>>     ever other mechanism you like. This
>> could also
>>>>>  (commonly)
>>>>>>>   cover
>>>>>>>>>    cases like
>>>>>>>>>>>>     detecting http UserAgents which are
>> not able to
>>>>>  handle
>>>>>>>   zipped
>>>>>>>>>    resources,
>>>>>>>>>>>>     etc. That way we could provide this
>> logic ONCE
>>>>>  and have
>>>>>>>   complete
>>>>>>>>>    freedom
>>>>>>>>>>>>     over the configuration.
>>>>>>>>>>>>
>>>>>>>>>>>>     B.) code reusable components once
>> and use them in
>>>>>  other
>>>>>>>   projects
>>>>>>>>>    (ev via
>>>>>>>>>>>>     shading it in).
>>>>>>>>>>>>     ClassLoaderResourceLoader would be a
>> perfect
>>>>>  candidate! I
>>>>>>>   grepped
>>>>>>>>>    through
>>>>>>>>>>>>     only the few pits which I have
>> checked out
>>>>>  locally and
>>>>>>>   found this
>>>>>>>>>    class 7
>>>>>>>>>>>>     SEVEN times! I just can't
>> believe that we
>>>>>  can't
>>>>>>>   move this
>>>>>>>>>    stuff to a shared
>>>>>>>>>>>>     modul...
>>>>>>>>>>>>
>>>>>>>>>>>>     Same for FacesServletMapping. 6
>> times copied
>>>>>  around,
>>>>>>>>>>>>     WebConfigProviderFactory 5 times,
>> ...
>>>>>>>>>>>>     There are whole packages with 10++
>> classes which
>>>>>  got
>>>>>>>   copied 1:1!
>>>>>>>>>>>>
>>>>>>>>>>>>     I really could cry seeing this :(
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>     What can we do to solve this?
>>>>>>>>>>>>
>>>>>>>>>>>>     Theoretically myfaces-shared should
>> contain this
>>>>>  stuff.
>>>>>>>   This is
>>>>>>>>>    exactly what
>>>>>>>>>>>>     it is for!
>>>>>>>>>>>>     Historically there have been some
>> hand forged
>>>>>  tweeks and
>>>>>>>   ugly
>>>>>>>>>    hacks, but
>>>>>>>>>>>>     nowadays we have the
>> maven-shade-plugin to make
>>>>>  our live
>>>>>>>   easier.
>>>>>>>>>>>>
>>>>>>>>>>>>     So the suggestion is:
>>>>>>>>>>>>
>>>>>>>>>>>>     1.) cleanup myfaces-shared.
>> mf-shared has almost
>>>>>  no
>>>>>>>   checkstyle
>>>>>>>>>    rules
>>>>>>>>>>>>     applied.
>>>>>>>>>>>>     2.) add unit tests for
>> myfaces-shared. Currently
>>>>>  there are
>>>>>>>   not
>>>>>>>>>    many...
>>>>>>>>>>>>     3.) move the shared code parts back
>> to
>>>>>  myfaces-shared and
>>>>>>>   add unit
>>>>>>>>>    tests.
>>>>>>>>>>>>     4.) import myfaces-shared via maven
>> dependency
>>>>>  and use
>>>>>>>>>    <minimizeJar> and
>>>>>>>>>>>>     <relocations> to package the
>> stuff
>>>>>>>>>>>>
>>>>>>>>>>>>     [+1] fine go ahead (ideally: yes,
>> what parts can
>>>>>  I help
>>>>>>>   with?)
>>>>>>>>>>>>     [0] dont care
>>>>>>>>>>>>     [-1] wont work because ...
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>     I've attached a file which
>> contains all
>>>>>  Classes which
>>>>>>>   name
>>>>>>>>>    exists multiple
>>>>>>>>>>>>     times in MyFaces. The number is the
>> cound how
>>>>>  often they
>>>>>>>   exist in
>>>>>>>>>    MyFaces. I
>>>>>>>>>>>>     excluded current20.
>>>>>>>>>>>>     Please note that classes with the
>> same name do
>>>>>  not
>>>>>>>   necessarily have
>>>>>>>>>    the same
>>>>>>>>>>>>     content - but quite a lot actually
>> do have!
>>>>>  (scroll to the
>>>>>>>   bottom
>>>>>>>>>    of the
>>>>>>>>>>>>     file ...)
>>>>>>>>>>>>
>>>>>>>>>>>>     LieGrue,
>>>>>>>>>>>>     strub
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>     --
>>>>>>>>>>>     Jakob Korherr
>>>>>>>>>>>
>>>>>>>>>>>     blog: http://www.jakobk.com
>>>>>>>>>>>     twitter: http://twitter.com/jakobkorherr
>>>>>>>>>>>     work: http://www.irian.at
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: [DISCUSS] how to get rid of tons of duplicated code

[Mark Struberg <st...@yahoo.de>]

Oki, just found time to do a  review:

1.) why do we need an own 'parent' module? I think all the basic plugin definitions should stay the same for all myfaces-core project, don't they?

2.)
> shared-utils : utilities for JSF used in core, but built as an api.
what do you mean with 'built as an api' ?


3.)
> Now, we move the duplicate code related to myfaces-commons-utils 

> into this module
+1 to dropping myfaces-commons-utils. This is just a 1:1 fork from shared.
Could you please explain again where you like to move/copy this parts to now?

4.) 
> and shared will have shared-utils as dependency
Which 'shared-utils' do you mean? the newly proposed module myfaces/core/shared ?
Actually myfaces/shared is then not needed anymore, right? This would be a pretty big change to what has been. 

To be honest: I'd rather make the unique source of all those things in myfaces/shared which it has been for years until only recently as this only got changed in July or August this year! I really just see no benefit in keeping this at myfaces/core/ over having it well maintained in myfaces/shared.
If we do that, we also don't need any ugly hacks while doing async releases - that's exactly what such a library is designed for

5.)
> Then we kill shared-tomahawk, and we make tomahawk use 
> maven-shade-plugin ..
 
 +1 
Actually this will be the biggest amount of work. To get rid of and unify all the copied sources again. 
But I really think this is way worth it!


Summary:

I think we now agree that we should aim to 'unify' our code which got copied multiple times and pull them back to a common usable library, right?

We are not yet d'accord if this common library should reside in myfaces/core/shared (as part of MyFaces core) or myfaces/shared (as a separate MyFaces subproject). What about listing pros and cons and then start a vote?


txs and LieGrue,
strub


PS: no worry, together we will soon solve this riddle ;)



----- Original Message -----
> From: Leonardo Uribe <lu...@gmail.com>
> To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg <st...@yahoo.de>
> Cc: 
> Sent: Tuesday, October 25, 2011 4:02 AM
> Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
> 
> Hi
> 
> Ok, it is done. In the branch it is possible to see how core will look
> after the change. The other changes are just use maven-shade plugin
> over myfaces-impl-shared-utils and myfaces-impl-shared internal
> modules, and do some package relocation in some cases.
> 
> regards
> 
> Leonardo Uribe
> 
> 2011/10/24 Leonardo Uribe <lu...@gmail.com>:
>>  Hi
>> 
>>  I started a branch to try it:
>> 
>> 
> http://svn.apache.org/repos/asf/myfaces/core/branches/2.0.x_refactor_shade_duplicate/
>> 
>>  just to gain some time. I think it is worth at least to try a prototype.
>> 
>>  regards,
>> 
>>  Leonardo Uribe
>> 
>>  2011/10/24 Mark Struberg <st...@yahoo.de>:
>>>  Hi Leo!
>>> 
>>>  Yes, this sounds much better, but please give me 2 days to think 
> through it in detail.
>>> 
>>>  txs and LieGrue,
>>>  strub
>>> 
>>> 
>>> 
>>>  ----- Original Message -----
>>>>  From: Leonardo Uribe <lu...@gmail.com>
>>>>  To: MyFaces Development <de...@myfaces.apache.org>; Mark 
> Struberg <st...@yahoo.de>
>>>>  Cc:
>>>>  Sent: Monday, October 24, 2011 7:18 PM
>>>>  Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>>> 
>>>>  Hi
>>>> 
>>>>  Ok, it is evident we have different opinions here about how to deal
>>>>  with this problem. So, rather than refute the arguments I think it 
> is
>>>>  more productive to show another proposal. In MyFaces Core 2.0.x we
>>>>  have the following layout:
>>>> 
>>>>  api/
>>>>  assembly/
>>>>  bundle/
>>>>  impl/
>>>>  implee6/
>>>>  integration-tests/
>>>>  pom.xml
>>>>  shared/
>>>>  src/
>>>> 
>>>>  Let's add two modules
>>>> 
>>>>  parent : contains the parent POM that all submodules should 
> inherit.
>>>>  shared-utils : utilities for JSF used in core, but built as an api.
>>>> 
>>>>  Now, we move the duplicate code related to myfaces-commons-utils 
> into
>>>>  this module, and shared will have shared-utils as dependency. impl
>>>>  module will use maven-shade plugin to take the code from 
> shared-utils
>>>>  and shared (actually this is done for only for shared).
>>>> 
>>>>  When a release of myfaces-core is done, all modules are released, 
> so
>>>>  things go as usual. But have the parent as module has the advantage
>>>>  that if we want to release shared-utils or shared internal modules, 
> we
>>>>  can do it only releasing parent (optional) and shared-utils.
>>>> 
>>>>  Since we can create a release for these modules, we remove the hack
>>>>  used on on shared project (hard copy). Just we modify the pom to 
> use
>>>>  maven-shade-plugin over myfaces-core shared module. Then we kill
>>>>  shared-tomahawk, and we make tomahawk use maven-shade-plugin over
>>>>  shaded artifact in shared project.
>>>> 
>>>>  In commons we do the same as in shared. In
>>>>  myfaces-commons-resourcehandler we use myfaces core shared module
>>>>  using maven-shade-plugin.
>>>> 
>>>>  The only disadvantage is the release process gets a little bit more
>>>>  complicated, but since do a release becomes easier with nexus, it 
> is
>>>>  ok.
>>>> 
>>>>  Do you agree with this solution?
>>>> 
>>>>  regards,
>>>> 
>>>>  Leonardo Uribe
>>>> 
>>>>  2011/10/24 Mark Struberg <st...@yahoo.de>:
>>>>>>  Really that was not solved using maven-shade-plugin.
>>>>>   Then we used the maven-shade-plugin the wrong way. See the
>>>>  <relocations> option [1].
>>>>> 
>>>>>>   There, there is a profile called
>>>>>>   "synch-myfaces-impl-shared", when it is added, 
> the code is
>>>>  copied and
>>>>>>   then a manual commit do the trick.
>>>>> 
>>>>>   I think this is an ugly hack and doesn't solve any 
> problems because
>>>>> 
>>>>>   a.)
>>>>>>   Take into account
>>>>>>   that each release requires a vote and that vote takes 3 
> days to get
>>>>>>   fixed.
>>>>>   you could just do a mvn release of shared + core in 1 go to 
> the same
>>>>  staging repo -> only 1 vote is needed!
>>>>>   This argument is simply wrong.
>>>>> 
>>>>>   b.)
>>>>>   You
>>>>>    currently copy the code over 1:1 (half manually) thus your 
> argument
>>>>>   with 'core and other projects need different sources' 
> is just nil.
>>>>  There
>>>>>    is no difference if you do this by profile, by hand or 
> automatically!
>>>>>   So I really prefer to have this automatically. Which is 
> exactly what a
>>>>>   dependency does...
>>>>> 
>>>>>   c.) the TCK argument is moot because it has
>>>>>   nothing to do with shared. Most of the issues in the TCK are 
> not
>>>>>   affecting shared. And if they do, then those fixes are needed 
> BY ALL
>>>>>   OTHER PROJECTS AS WELL. Thus another argument against hiding 
> this code
>>>>>   and duplicating it all over...
>>>>> 
>>>>>   c.)
>>>>>>   Instead, maybe the option is reorganize myfaces core to 
> allow
>>>>>>   alternate release lifecycles per module
>>>>>   Sorry I don't grok this argument. It sounds as it would 
> make all things
>>>>  more complicated without solving any real problem.
>>>>> 
>>>>>   e.)
>>>>>>   This means myfaces-commons project should be 
> "merged" in some
>>>>  way with
>>>>>>   myfaces core. It has sense.
>>>>>   2 options:
>>>>>   1..) kill myfaces-shared completely and use the shared from 
> core instead.
>>>>>   Downside: if you need some fix in the shared code for some 
> other project,
>>>>  you would need to release mf-core
>>>>>   2.) kill the newly introduced (this got only created a few 
> weeks back by
>>>>  you) core-shared and use mf-shared again.
>>>>>   Downside: hmmm nothing if one understands how to release 
> correctly.
>>>>> 
>>>>>   f.)
>>>>>    all your explanations only explain the duplication between
>>>>>   myfaces-shared and myfaces-core-shared. I can live with the 
> duplication
>>>>>   for now, but we also have classes which got copied around up 
> to 8 times!
>>>>>    There is no excuse for that imo.
>>>>> 
>>>>>   g.)
>>>>>>   But what happen when you have some code that does not have 
> a clear
>>>>>>   "interface". If somebody removes or change some 
> code because
>>>>  he/she
>>>>>>   thinks it is not used in core or whatever, all 6 projects 
> that could
>>>>>>   require it will be affected and will require to rework its 
> code.
>>>>>>   Things get uglier when you have one library working with 
> version 1.1.1
>>>>>>   and 1.1.2 is binary incompatible with version 1.1.1, but 
> my other
>>>>>>   dependency requires it and kaboooom, the application does 
> not work.
>>>>>>   So, the first assumption we need to preserve in those
>>>>  "shared"
>>>>>>   artifacts is build it as an API, preserving binary 
> compatibility.
>>>>> 
>>>>>   I don't get that argument neither!
>>>>>   Hey,
>>>>>    that's life! If it turns out that the code is not good 
> enough and
>>>>  needs
>>>>>    a fix, then that's the way it is! All other projects 
> should fix that
>>>>>   too in that case. I rather have a reproducible compile error 
> which
>>>>>   easily could get fixed than having tons of duplicated code 
> which is more
>>>>>    or less always logically broken and badly tested.
>>>>>   Yes, we should be
>>>>>   aware that the classes we put into myfaces-shared must meet 
> some
>>>>>   standards and need to be well tested. But actually that would 
> benefit
>>>>>   our project a lot.
>>>>> 
>>>>> 
>>>>>   h.) I just realised that our process in copying shared-impl 
> from core to
>>>>  mf-shared is even more broken than every process before.
>>>>>   If you are working on a lets say mf-commons project and find a 
> bug in any
>>>>  of those shared parts, then you would need to RELEASE MF-CORE 
> FIRST? omg, this
>>>>  cannot be serious!
>>>>> 
>>>>> 
>>>>>   LieGrue,
>>>>>   strub
>>>>> 
>>>>> 
>>>>>   [1]
>>>> 
> http://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#relocations
>>>>> 
>>>>> 
>>>>> 
>>>>>   ----- Original Message -----
>>>>>>   From: Leonardo Uribe <lu...@gmail.com>
>>>>>>   To: MyFaces Development <de...@myfaces.apache.org>; 
> Mark Struberg
>>>>  <st...@yahoo.de>
>>>>>>   Cc:
>>>>>>   Sent: Monday, October 24, 2011 4:36 AM
>>>>>>   Subject: Re: [DISCUSS] how to get rid of tons of 
> duplicated code
>>>>>> 
>>>>>>   Hi
>>>>>> 
>>>>>>   2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>>>>>    I've now read through the old mail archives and 
> understand
>>>>  what the
>>>>>>   original problem was. But actually I don't think we 
> solved it
>>>>  correctly
>>>>>>   right now. Of course we solved to original problem, but 
> opened a can of
>>>>  worms
>>>>>>   causing other problems.
>>>>>>> 
>>>>>>>    The problem as far as I remember has been that 
> myfaces-shared had
>>>>  tons of
>>>>>>   duplicated code in it. One for core, one for tomahawk, one 
> for
>>>>  trinidad, etc.
>>>>>>> 
>>>>>>> 
>>>>>>>    The shared part for core got moved to myfaces-core, 
> but the deeper
>>>>  problem
>>>>>>   was that it was not easily possible to have multiple 
> different versions
>>>>  of
>>>>>>   myfaces-shared. This now got solved by using the 
> maven-shade-plugin. So
>>>>  we
>>>>>>   should rethink the practice to duplicate all the code and 
> aim for a
>>>>  _clean_
>>>>>>   solution.
>>>>>>> 
>>>>>> 
>>>>>>   Really that was not solved using maven-shade-plugin. What 
> we did was
>>>>>>   copy the code into myfaces-core and create a mirror of the 
> same code
>>>>>>   under shared. There, there is a profile called
>>>>>>   "synch-myfaces-impl-shared", when it is added, 
> the code is
>>>>  copied and
>>>>>>   then a manual commit do the trick.
>>>>>> 
>>>>>>>    Also (being a maven guy) I cannot quite follow the 
> argument about
>>>>  the
>>>>>>   release cycles. Running a myfaces-shared release and then 
> (with the
>>>>  same staging
>>>>>>   repo) a myfaces-core release is a task of 15 minutes. + 
> the time for
>>>>  running the
>>>>>>   TCK, but this gets run via CI anyway, right? Thus this is 
> barely a
>>>>  problem.
>>>>>>>    If it is then I'd happily volunteer to do the 
> next release (do
>>>>  this for
>>>>>>   a few projects already) As you know, performing a release 
> really got
>>>>  _much_
>>>>>>   easier nowadays with our new apache-parent pom.
>>>>>>>    But maybe this argument was only meant for our old 
> release process
>>>>  (which I
>>>>>>   agree was a lot of work)?
>>>>>>> 
>>>>>>>    If your answer is 'it's still needed' 
> then can we just
>>>>  unify
>>>>>>   all other usages?
>>>>>>> 
>>>>>> 
>>>>>>   Make a release is just the first of the problems. Take 
> into account
>>>>>>   that each release requires a vote and that vote takes 3 
> days to get
>>>>>>   fixed. So in practice a problem in core can effectively 
> block a
>>>>>>   release of other artifacts. That's very inconvenient. 
> Suppose we
>>>>  have
>>>>>>   a new TCK and that one found a problem on myfaces core. 
> Again even if
>>>>>>   the other artifacts are good enough, this becomes a 
> blocker. There are
>>>>>>   enough historical evidence that supports this point. In 
> conclusion
>>>>>>   this slow down the whole release cycle we have on myfaces. 
> So ignore
>>>>>>   that is not an option.
>>>>>> 
>>>>>>   Instead, maybe the option is reorganize myfaces core to 
> allow
>>>>>>   alternate release lifecycles per module. For example, each 
> maven
>>>>>>   plugin in myfaces has its own release lifecycle and there 
> is a parent
>>>>>>   pom with a different release procedure. This requires some 
> changes to
>>>>>>   create the source-release.zip file inherited from apache 
> pom. But it
>>>>>>   could be a cleaner solution.
>>>>>> 
>>>>>>   This means myfaces-commons project should be 
> "merged" in some
>>>>  way with
>>>>>>   myfaces core. It has sense.
>>>>>> 
>>>>>>>    One question which bothers me with the 
> 'shared' approach
>>>>  if what
>>>>>>   would happen to our build-tools annotation scanning
>>>>  (@JSFWebConfigParam, etc)?
>>>>>>   Does this already work with dependencies? Do we have this 
> problem
>>>>  already due to
>>>>>>   the fact that we import such annotated classes via 
> dependency?
>>>>>>> 
>>>>>> 
>>>>>>   Those annotations comes from myfaces-builder-annotations. 
> They are
>>>>>>   source code annotations but all that information are saved 
> on
>>>>>>   myfaces-metadata.xml, so even if dissapear on compile 
> time, the
>>>>>>   information can be gathered from there. It is not a 
> problem.
>>>>>> 
>>>>>>>>    Additionally, we increase the risk of "side
>>>>  effects",
>>>>>>>>    because a change done in core could introduce a 
> bug in other
>>>>  parts.
>>>>>>>    Imo it's exactly the opposite. If you use the 
> same code in 7
>>>>  projects,
>>>>>>   then it is more likely that a bug gets found and fixed.
>>>>>>>    And the opposite case is (sadly) absolutely unlikely. 
> If you have
>>>>  a class
>>>>>>   duplicated 7 times and find a bug in one project, it is 
> highly unlikely
>>>>  that all
>>>>>>   6 other projects will get this fix applied :(
>>>>>>> 
>>>>>> 
>>>>>>   But what happen when you have some code that does not have 
> a clear
>>>>>>   "interface". If somebody removes or change some 
> code because
>>>>  he/she
>>>>>>   thinks it is not used in core or whatever, all 6 projects 
> that could
>>>>>>   require it will be affected and will require to rework its 
> code.
>>>>>>   Things get uglier when you have one library working with 
> version 1.1.1
>>>>>>   and 1.1.2 is binary incompatible with version 1.1.1, but 
> my other
>>>>>>   dependency requires it and kaboooom, the application does 
> not work.
>>>>>>   So, the first assumption we need to preserve in those
>>>>  "shared"
>>>>>>   artifacts is build it as an API, preserving binary 
> compatibility.
>>>>>> 
>>>>>>   So we can't just grab the code from shared as is and 
> say to users
>>>>  "...
>>>>>>   you can use that into its own projects ...". If the 
> project is
>>>>>>   maintained inside myfaces we can fix such problems, but 
> outside
>>>>>>   myfaces we should be more strict. So, we need a 
> "public
>>>>  shared" code
>>>>>>   like the one proposed in myfaces commons and other code 
> "myfaces
>>>>>>   shared" to use in projects like tomahawk or 
> portletbridge or
>>>>  whatever
>>>>>>   inside our land.
>>>>>> 
>>>>>>   regards,
>>>>>> 
>>>>>>   Leonardo Uribe
>>>>>> 
>>>>>>>    LieGrue,
>>>>>>>    strub
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>>    ----- Original Message -----
>>>>>>>>    From: Leonardo Uribe <lu...@gmail.com>
>>>>>>>>    To: MyFaces Development 
> <de...@myfaces.apache.org>
>>>>>>>>    Cc: Mark Struberg <st...@yahoo.de>
>>>>>>>>    Sent: Sunday, October 23, 2011 9:08 PM
>>>>>>>>    Subject: Re: [DISCUSS] how to get rid of tons of 
> duplicated
>>>>  code
>>>>>>>> 
>>>>>>>>    Hi
>>>>>>>> 
>>>>>>>>    Ok, let's check the proposal
>>>>>>>> 
>>>>>>>>    MS>> So the suggestion is:
>>>>>>>>    MS>>
>>>>>>>>    MS>> 1.) cleanup myfaces-shared. mf-shared 
> has almost no
>>>>>>   checkstyle
>>>>>>>>    rules applied.
>>>>>>>> 
>>>>>>>>    Yes, sounds good.
>>>>>>>> 
>>>>>>>>    MS>> 2.) add unit tests for myfaces-shared. 
> Currently
>>>>  there are
>>>>>>   not
>>>>>>>>    many...
>>>>>>>> 
>>>>>>>>    Ok, sounds good too.
>>>>>>>> 
>>>>>>>>    MS>> 3.) move the shared code parts back to
>>>>  myfaces-shared and
>>>>>>   add unit
>>>>>>>>    tests.
>>>>>>>> 
>>>>>>>>    So, this means do one step back and move the code 
> from
>>>>  myfaces-core
>>>>>>>>    "shared" to myfaces-shared project? 
> This breaks
>>>>  effectively
>>>>>>   the
>>>>>>>>    changes done some months ago to make easier work 
> with myfaces
>>>>  core
>>>>>>>>    itself.
>>>>>>>> 
>>>>>>>>    In that time the conclusion was: "core has 
> priority over
>>>>  anything
>>>>>>>>    else, so shared code must live in core, but 
> myfaces-shared
>>>>  project
>>>>>>>>    should just copy the code from there and have its 
> own
>>>>  lifecycle"
>>>>>>>>    (these are my own words as I understood).
>>>>>>>> 
>>>>>>>>    So this point does not have practical sense, and 
> go against
>>>>  everything
>>>>>>>>    discussed earlier.
>>>>>>>> 
>>>>>>>>    MS>> 4.) import myfaces-shared via maven 
> dependency and
>>>>  use
>>>>>>>>    <minimizeJar> and <relocations> to 
> package the
>>>>  stuff
>>>>>>>> 
>>>>>>>>    maven-shade-plugin is a good "tool" but 
> doesn't
>>>>  fit well
>>>>>>   in this
>>>>>>>>    scenario. The reason is we need an alternate 
> release lifecycle
>>>>  for the
>>>>>>>>    shared code between myfaces core and other 
> projects.
>>>>>>>> 
>>>>>>>>    Historically that was the very first intention 
> behind
>>>>  myfaces-shared
>>>>>>>>    project. Any myfaces core release requires some 
> additional
>>>>  steps to do
>>>>>>>>    (TCK), so that becomes a problem when you try to 
> release other
>>>>>>>>    libraries that depends of shared. So, to fix 
> that,
>>>>  "shared"
>>>>>>   was
>>>>>>>>    created, so the code can be released in a 
> independent way, and
>>>>  prevent
>>>>>>>>    myfaces core becomes an obstacle to release any 
> other project
>>>>>>>>    (tomahawk, portlet-bridge, ... ). So, to release 
> tomahawk you
>>>>  release
>>>>>>>>    shared first and then tomahawk.
>>>>>>>> 
>>>>>>>>    maven-shade-plugin requires a released artifact 
> to do its job.
>>>>  So, use
>>>>>>>>    it impose that restriction. In "shared" 
> case,
>>>>  preserve the
>>>>>>   original
>>>>>>>>    intention becomes "imperative", and 
> that's the
>>>>  reason why
>>>>>>   a goal
>>>>>>>>    was
>>>>>>>>    created to copy the code from myfaces-core 
> shared, so the
>>>>  release
>>>>>>>>    manager can run this goal, commit the changes and 
> then run a
>>>>  release.
>>>>>>>> 
>>>>>>>>    My proposal in this case is do the same we did 
> for shared, but
>>>>  for
>>>>>>>>    "myfaces commons" case. Then we can use
>>>>  maven-shade-plugin in
>>>>>>   other
>>>>>>>>    projects, but not over shared, instead over a 
> released version
>>>>  of
>>>>>>>>    myfaces-commons-utils. Keep tomahawk or 
> portlet-bridge as is,
>>>>  using
>>>>>>>>    shared project, because by its nature, those 
> projects require
>>>>  classes
>>>>>>>>    that are not meant to be used outside those 
> cases.
>>>>>>>> 
>>>>>>>>    Note do any hack in this part makes a little bit
>>>>  "obscure"
>>>>>>   how to make
>>>>>>>>    changes, because everything becomes 
> "centralized",
>>>>  but makes
>>>>>>   easier
>>>>>>>>    maintain code. Additionally, we increase the risk 
> of
>>>>  "side
>>>>>>   effects",
>>>>>>>>    because a change done in core could introduce a 
> bug in other
>>>>  parts. So
>>>>>>>>    at the end this is a matter of how to keep our 
> code
>>>>>>   "balanced", even
>>>>>>>>    if some times it becomes a decision about 
> "choose the
>>>>  less
>>>>>>>>    inconvenient alternative".
>>>>>>>> 
>>>>>>>>    regards,
>>>>>>>> 
>>>>>>>>    Leonardo Uribe
>>>>>>>> 
>>>>>>>>    2011/10/23 Leonardo Uribe 
> <lu...@gmail.com>:
>>>>>>>>>     Hi
>>>>>>>>> 
>>>>>>>>>     2011/10/23 Jakob Korherr 
> <ja...@gmail.com>:
>>>>>>>>>>     Hi Mark,
>>>>>>>>>> 
>>>>>>>>>>     +1 - that's exactly what I have been 
> trying to
>>>>  accomplish
>>>>>>   some time
>>>>>>>>>>     ago (introducing common-shades [1]). 
> Unfortunately, I
>>>>  was not
>>>>>>>>>>     successful back then.
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>>     It is clear we need to "split" 
> myfaces-impl
>>>>  into
>>>>>>   multiple
>>>>>>>>    modules. There
>>>>>>>>>     are some parts that are useful for other 
> projects. The
>>>>  code you
>>>>>>   did
>>>>>>>>>     on commons-shade was the attempt to solve 
> the problem of
>>>>  the
>>>>>>>>>     duplicate code used on myfaces-test.
>>>>>>>>> 
>>>>>>>>>     Now the objective is find a way about how to 
> reuse code
>>>>  in myfaces
>>>>>>>>>     core between multiple projects effectively.
>>>>>>>>> 
>>>>>>>>>>     However, there is a slight problem with 
> moving all
>>>>  this stuff
>>>>>>   into
>>>>>>>>>>     MyFaces shared, which I want to point 
> out: code size.
>>>>  If we
>>>>>>   really put
>>>>>>>>>>     all the code that is shared across any 
> MyFaces
>>>>  subproject into
>>>>>>   shared,
>>>>>>>>>>     it will get fat and ugly (even more than 
> it is right
>>>>  now). In
>>>>>>>>>>     addition, if we continue including the 
> whole shared
>>>>  project
>>>>>>   into
>>>>>>>>>>     MyFaces core, MyFaces core impl will get 
> bigger and
>>>>  bigger.
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>>     Yes, the problem basically is MyFaces shared 
> does not
>>>>  have any
>>>>>>   order
>>>>>>>>>     or any notion of API. There are code that is 
> used only in
>>>>  tomahawk
>>>>>>   but not
>>>>>>>>>     intended to use in any other place. There 
> are some useful
>>>>>>   utitlities but
>>>>>>>>>     sometimes without documentation, and there 
> are some other
>>>>  code
>>>>>>   that is
>>>>>>>>>     just obsolete. It it clear a cleanup of that 
> location is
>>>>>>>>>     necessary, but note priorities comes first, 
> so this task
>>>>  has been
>>>>>>   delayed
>>>>>>>>    in
>>>>>>>>>     order to deal with other important stuff. 
> Now it is a
>>>>  good time to
>>>>>>   fix
>>>>>>>>    this.
>>>>>>>>> 
>>>>>>>>>>     Thus I'd like to suggest something 
> similar which
>>>>  I wanted
>>>>>>   to
>>>>>>>>>>     accomplish with common-shades: Introduce 
> a new shared
>>>>  module,
>>>>>>   which
>>>>>>>>>>     consists of many submodules that each 
> handle a
>>>>  specific
>>>>>>   functionality
>>>>>>>>>>     instead of being one fat module. With 
> this approach
>>>>  each
>>>>>>   MyFaces
>>>>>>>>>>     subproject would be able to pick out 
> only the stuff
>>>>  it really
>>>>>>   needs.
>>>>>>>>>>     Furthermore we would see more easily 
> which code in
>>>>  shared is
>>>>>>   not used
>>>>>>>>>>     anymore (I guess at the moment there is 
> a lot of it),
>>>>  just by
>>>>>>   checking
>>>>>>>>>>     which modules are still used in our 
> poms.
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>>     That is the big question, how to split 
> myfaces-impl and
>>>>  shared.
>>>>>>   Precisely
>>>>>>>>>     the intention of myfaces-commons-utils 
> projects was take
>>>>  the stuff
>>>>>>   that is
>>>>>>>>>     useful from shared and build an usable API 
> for developers
>>>>  outside
>>>>>>   MyFaces.
>>>>>>>>> 
>>>>>>>>>     For example, MyFaces HTML5 subproject was a 
> good
>>>>  experiment to see
>>>>>>>>>     which code is useful and should be added in 
> a API. Some
>>>>  weeks ago
>>>>>>   I checked
>>>>>>>>>     and removed all duplicate code to use
>>>>  myfaces-commons-utils. So
>>>>>>   the 1.0.2
>>>>>>>>>     release contains those classes taken from 
> shared.
>>>>>>>>> 
>>>>>>>>>     regards,
>>>>>>>>> 
>>>>>>>>>     Leonardo Uribe
>>>>>>>>> 
>>>>>>>>>>     Regards,
>>>>>>>>>>     Jakob
>>>>>>>>>> 
>>>>>>>>>>     [1]
>>>>  https://svn.apache.org/repos/asf/myfaces/common-shades/
>>>>>>>>>> 
>>>>>>>>>>     2011/10/23 Mark Struberg 
> <st...@yahoo.de>:
>>>>>>>>>>>     Hi!
>>>>>>>>>>>     While working on the mafyces-commons 
> cleanup I
>>>>  figured
>>>>>>   that we have
>>>>>>>>    tons of
>>>>>>>>>>>     duplicated code spread over MyFaces.
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>>     As an example I like to mention
>>>>>>   myfaces-commons-resourcehandler.
>>>>>>>>    There are
>>>>>>>>>>>     43 classes in total, and 35 of them 
> are just 1:1
>>>>  copied
>>>>>>   from other
>>>>>>>>    projects
>>>>>>>>>>>     to provide resource management, zip, 
> etc. For me
>>>>  this is
>>>>>>   an
>>>>>>>>    absolute no-go.
>>>>>>>>>>>     Those classes have neither tests nor 
> any
>>>>  documentation
>>>>>>   where they
>>>>>>>>    got forked
>>>>>>>>>>>     from. Nor will any bug which gets 
> fixed in
>>>>  another module
>>>>>>   make
>>>>>>>>    it's way over
>>>>>>>>>>>     to all the other projects containing 
> that very
>>>>  forked
>>>>>>   code.
>>>>>>>>    That's just ...
>>>>>>>>>>>     unbelievable unmaintainable.
>>>>>>>>>>> 
>>>>>>>>>>>     There are 2 different ways to solve 
> this
>>>>  (depending on the
>>>>>>>>    problem):
>>>>>>>>>>> 
>>>>>>>>>>>     A.) drop the functionality and 
> provide a
>>>>  generalized
>>>>>>   solution. The
>>>>>>>>    GZIP of
>>>>>>>>>>>     myfaces-commons-resourcehandleris an 
> obvious
>>>>  example:
>>>>>>>>>>>     We now copy this code over the 4th 
> time or even
>>>>  more.
>>>>>>   Instead of
>>>>>>>>    doing this,
>>>>>>>>>>>     we should rather do it in the 
> classic unix
>>>>  fashion: do one
>>>>>>   thing,
>>>>>>>>    but do it
>>>>>>>>>>>     well.
>>>>>>>>>>>     Which means I'd rather see all 
> the GZIP stuff
>>>>  factored
>>>>>>   out into
>>>>>>>>    an own
>>>>>>>>>>>     mf-commons module as a Servlet 
> Filter. This can
>>>>  then get
>>>>>>   applied to
>>>>>>>>    what
>>>>>>>>>>>     ever other mechanism you like. This 
> could also
>>>>  (commonly)
>>>>>>   cover
>>>>>>>>    cases like
>>>>>>>>>>>     detecting http UserAgents which are 
> not able to
>>>>  handle
>>>>>>   zipped
>>>>>>>>    resources,
>>>>>>>>>>>     etc. That way we could provide this 
> logic ONCE
>>>>  and have
>>>>>>   complete
>>>>>>>>    freedom
>>>>>>>>>>>     over the configuration.
>>>>>>>>>>> 
>>>>>>>>>>>     B.) code reusable components once 
> and use them in
>>>>  other
>>>>>>   projects
>>>>>>>>    (ev via
>>>>>>>>>>>     shading it in).
>>>>>>>>>>>     ClassLoaderResourceLoader would be a 
> perfect
>>>>  candidate! I
>>>>>>   grepped
>>>>>>>>    through
>>>>>>>>>>>     only the few pits which I have 
> checked out
>>>>  locally and
>>>>>>   found this
>>>>>>>>    class 7
>>>>>>>>>>>     SEVEN times! I just can't 
> believe that we
>>>>  can't
>>>>>>   move this
>>>>>>>>    stuff to a shared
>>>>>>>>>>>     modul...
>>>>>>>>>>> 
>>>>>>>>>>>     Same for FacesServletMapping. 6 
> times copied
>>>>  around,
>>>>>>>>>>>     WebConfigProviderFactory 5 times, 
> ...
>>>>>>>>>>>     There are whole packages with 10++ 
> classes which
>>>>  got
>>>>>>   copied 1:1!
>>>>>>>>>>> 
>>>>>>>>>>>     I really could cry seeing this :(
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>>     What can we do to solve this?
>>>>>>>>>>> 
>>>>>>>>>>>     Theoretically myfaces-shared should 
> contain this
>>>>  stuff.
>>>>>>   This is
>>>>>>>>    exactly what
>>>>>>>>>>>     it is for!
>>>>>>>>>>>     Historically there have been some 
> hand forged
>>>>  tweeks and
>>>>>>   ugly
>>>>>>>>    hacks, but
>>>>>>>>>>>     nowadays we have the 
> maven-shade-plugin to make
>>>>  our live
>>>>>>   easier.
>>>>>>>>>>> 
>>>>>>>>>>>     So the suggestion is:
>>>>>>>>>>> 
>>>>>>>>>>>     1.) cleanup myfaces-shared. 
> mf-shared has almost
>>>>  no
>>>>>>   checkstyle
>>>>>>>>    rules
>>>>>>>>>>>     applied.
>>>>>>>>>>>     2.) add unit tests for 
> myfaces-shared. Currently
>>>>  there are
>>>>>>   not
>>>>>>>>    many...
>>>>>>>>>>>     3.) move the shared code parts back 
> to
>>>>  myfaces-shared and
>>>>>>   add unit
>>>>>>>>    tests.
>>>>>>>>>>>     4.) import myfaces-shared via maven 
> dependency
>>>>  and use
>>>>>>>>    <minimizeJar> and
>>>>>>>>>>>     <relocations> to package the 
> stuff
>>>>>>>>>>> 
>>>>>>>>>>>     [+1] fine go ahead (ideally: yes, 
> what parts can
>>>>  I help
>>>>>>   with?)
>>>>>>>>>>>     [0] dont care
>>>>>>>>>>>     [-1] wont work because ...
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>>     I've attached a file which 
> contains all
>>>>  Classes which
>>>>>>   name
>>>>>>>>    exists multiple
>>>>>>>>>>>     times in MyFaces. The number is the 
> cound how
>>>>  often they
>>>>>>   exist in
>>>>>>>>    MyFaces. I
>>>>>>>>>>>     excluded current20.
>>>>>>>>>>>     Please note that classes with the 
> same name do
>>>>  not
>>>>>>   necessarily have
>>>>>>>>    the same
>>>>>>>>>>>     content - but quite a lot actually 
> do have!
>>>>  (scroll to the
>>>>>>   bottom
>>>>>>>>    of the
>>>>>>>>>>>     file ...)
>>>>>>>>>>> 
>>>>>>>>>>>     LieGrue,
>>>>>>>>>>>     strub
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>>     --
>>>>>>>>>>     Jakob Korherr
>>>>>>>>>> 
>>>>>>>>>>     blog: http://www.jakobk.com
>>>>>>>>>>     twitter: http://twitter.com/jakobkorherr
>>>>>>>>>>     work: http://www.irian.at
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>

Re: [DISCUSS] how to get rid of tons of duplicated code

[Leonardo Uribe <lu...@gmail.com>]

Hi

Ok, it is done. In the branch it is possible to see how core will look
after the change. The other changes are just use maven-shade plugin
over myfaces-impl-shared-utils and myfaces-impl-shared internal
modules, and do some package relocation in some cases.

regards

Leonardo Uribe

2011/10/24 Leonardo Uribe <lu...@gmail.com>:
> Hi
>
> I started a branch to try it:
>
> http://svn.apache.org/repos/asf/myfaces/core/branches/2.0.x_refactor_shade_duplicate/
>
> just to gain some time. I think it is worth at least to try a prototype.
>
> regards,
>
> Leonardo Uribe
>
> 2011/10/24 Mark Struberg <st...@yahoo.de>:
>> Hi Leo!
>>
>> Yes, this sounds much better, but please give me 2 days to think through it in detail.
>>
>> txs and LieGrue,
>> strub
>>
>>
>>
>> ----- Original Message -----
>>> From: Leonardo Uribe <lu...@gmail.com>
>>> To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg <st...@yahoo.de>
>>> Cc:
>>> Sent: Monday, October 24, 2011 7:18 PM
>>> Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>>
>>> Hi
>>>
>>> Ok, it is evident we have different opinions here about how to deal
>>> with this problem. So, rather than refute the arguments I think it is
>>> more productive to show another proposal. In MyFaces Core 2.0.x we
>>> have the following layout:
>>>
>>> api/
>>> assembly/
>>> bundle/
>>> impl/
>>> implee6/
>>> integration-tests/
>>> pom.xml
>>> shared/
>>> src/
>>>
>>> Let's add two modules
>>>
>>> parent : contains the parent POM that all submodules should inherit.
>>> shared-utils : utilities for JSF used in core, but built as an api.
>>>
>>> Now, we move the duplicate code related to myfaces-commons-utils into
>>> this module, and shared will have shared-utils as dependency. impl
>>> module will use maven-shade plugin to take the code from shared-utils
>>> and shared (actually this is done for only for shared).
>>>
>>> When a release of myfaces-core is done, all modules are released, so
>>> things go as usual. But have the parent as module has the advantage
>>> that if we want to release shared-utils or shared internal modules, we
>>> can do it only releasing parent (optional) and shared-utils.
>>>
>>> Since we can create a release for these modules, we remove the hack
>>> used on on shared project (hard copy). Just we modify the pom to use
>>> maven-shade-plugin over myfaces-core shared module. Then we kill
>>> shared-tomahawk, and we make tomahawk use maven-shade-plugin over
>>> shaded artifact in shared project.
>>>
>>> In commons we do the same as in shared. In
>>> myfaces-commons-resourcehandler we use myfaces core shared module
>>> using maven-shade-plugin.
>>>
>>> The only disadvantage is the release process gets a little bit more
>>> complicated, but since do a release becomes easier with nexus, it is
>>> ok.
>>>
>>> Do you agree with this solution?
>>>
>>> regards,
>>>
>>> Leonardo Uribe
>>>
>>> 2011/10/24 Mark Struberg <st...@yahoo.de>:
>>>>> Really that was not solved using maven-shade-plugin.
>>>>  Then we used the maven-shade-plugin the wrong way. See the
>>> <relocations> option [1].
>>>>
>>>>>  There, there is a profile called
>>>>>  "synch-myfaces-impl-shared", when it is added, the code is
>>> copied and
>>>>>  then a manual commit do the trick.
>>>>
>>>>  I think this is an ugly hack and doesn't solve any problems because
>>>>
>>>>  a.)
>>>>>  Take into account
>>>>>  that each release requires a vote and that vote takes 3 days to get
>>>>>  fixed.
>>>>  you could just do a mvn release of shared + core in 1 go to the same
>>> staging repo -> only 1 vote is needed!
>>>>  This argument is simply wrong.
>>>>
>>>>  b.)
>>>>  You
>>>>   currently copy the code over 1:1 (half manually) thus your argument
>>>>  with 'core and other projects need different sources' is just nil.
>>> There
>>>>   is no difference if you do this by profile, by hand or automatically!
>>>>  So I really prefer to have this automatically. Which is exactly what a
>>>>  dependency does...
>>>>
>>>>  c.) the TCK argument is moot because it has
>>>>  nothing to do with shared. Most of the issues in the TCK are not
>>>>  affecting shared. And if they do, then those fixes are needed BY ALL
>>>>  OTHER PROJECTS AS WELL. Thus another argument against hiding this code
>>>>  and duplicating it all over...
>>>>
>>>>  c.)
>>>>>  Instead, maybe the option is reorganize myfaces core to allow
>>>>>  alternate release lifecycles per module
>>>>  Sorry I don't grok this argument. It sounds as it would make all things
>>> more complicated without solving any real problem.
>>>>
>>>>  e.)
>>>>>  This means myfaces-commons project should be "merged" in some
>>> way with
>>>>>  myfaces core. It has sense.
>>>>  2 options:
>>>>  1..) kill myfaces-shared completely and use the shared from core instead.
>>>>  Downside: if you need some fix in the shared code for some other project,
>>> you would need to release mf-core
>>>>  2.) kill the newly introduced (this got only created a few weeks back by
>>> you) core-shared and use mf-shared again.
>>>>  Downside: hmmm nothing if one understands how to release correctly.
>>>>
>>>>  f.)
>>>>   all your explanations only explain the duplication between
>>>>  myfaces-shared and myfaces-core-shared. I can live with the duplication
>>>>  for now, but we also have classes which got copied around up to 8 times!
>>>>   There is no excuse for that imo.
>>>>
>>>>  g.)
>>>>>  But what happen when you have some code that does not have a clear
>>>>>  "interface". If somebody removes or change some code because
>>> he/she
>>>>>  thinks it is not used in core or whatever, all 6 projects that could
>>>>>  require it will be affected and will require to rework its code.
>>>>>  Things get uglier when you have one library working with version 1.1.1
>>>>>  and 1.1.2 is binary incompatible with version 1.1.1, but my other
>>>>>  dependency requires it and kaboooom, the application does not work.
>>>>>  So, the first assumption we need to preserve in those
>>> "shared"
>>>>>  artifacts is build it as an API, preserving binary compatibility.
>>>>
>>>>  I don't get that argument neither!
>>>>  Hey,
>>>>   that's life! If it turns out that the code is not good enough and
>>> needs
>>>>   a fix, then that's the way it is! All other projects should fix that
>>>>  too in that case. I rather have a reproducible compile error which
>>>>  easily could get fixed than having tons of duplicated code which is more
>>>>   or less always logically broken and badly tested.
>>>>  Yes, we should be
>>>>  aware that the classes we put into myfaces-shared must meet some
>>>>  standards and need to be well tested. But actually that would benefit
>>>>  our project a lot.
>>>>
>>>>
>>>>  h.) I just realised that our process in copying shared-impl from core to
>>> mf-shared is even more broken than every process before.
>>>>  If you are working on a lets say mf-commons project and find a bug in any
>>> of those shared parts, then you would need to RELEASE MF-CORE FIRST? omg, this
>>> cannot be serious!
>>>>
>>>>
>>>>  LieGrue,
>>>>  strub
>>>>
>>>>
>>>>  [1]
>>> http://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#relocations
>>>>
>>>>
>>>>
>>>>  ----- Original Message -----
>>>>>  From: Leonardo Uribe <lu...@gmail.com>
>>>>>  To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg
>>> <st...@yahoo.de>
>>>>>  Cc:
>>>>>  Sent: Monday, October 24, 2011 4:36 AM
>>>>>  Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>>>>
>>>>>  Hi
>>>>>
>>>>>  2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>>>>   I've now read through the old mail archives and understand
>>> what the
>>>>>  original problem was. But actually I don't think we solved it
>>> correctly
>>>>>  right now. Of course we solved to original problem, but opened a can of
>>> worms
>>>>>  causing other problems.
>>>>>>
>>>>>>   The problem as far as I remember has been that myfaces-shared had
>>> tons of
>>>>>  duplicated code in it. One for core, one for tomahawk, one for
>>> trinidad, etc.
>>>>>>
>>>>>>
>>>>>>   The shared part for core got moved to myfaces-core, but the deeper
>>> problem
>>>>>  was that it was not easily possible to have multiple different versions
>>> of
>>>>>  myfaces-shared. This now got solved by using the maven-shade-plugin. So
>>> we
>>>>>  should rethink the practice to duplicate all the code and aim for a
>>> _clean_
>>>>>  solution.
>>>>>>
>>>>>
>>>>>  Really that was not solved using maven-shade-plugin. What we did was
>>>>>  copy the code into myfaces-core and create a mirror of the same code
>>>>>  under shared. There, there is a profile called
>>>>>  "synch-myfaces-impl-shared", when it is added, the code is
>>> copied and
>>>>>  then a manual commit do the trick.
>>>>>
>>>>>>   Also (being a maven guy) I cannot quite follow the argument about
>>> the
>>>>>  release cycles. Running a myfaces-shared release and then (with the
>>> same staging
>>>>>  repo) a myfaces-core release is a task of 15 minutes. + the time for
>>> running the
>>>>>  TCK, but this gets run via CI anyway, right? Thus this is barely a
>>> problem.
>>>>>>   If it is then I'd happily volunteer to do the next release (do
>>> this for
>>>>>  a few projects already) As you know, performing a release really got
>>> _much_
>>>>>  easier nowadays with our new apache-parent pom.
>>>>>>   But maybe this argument was only meant for our old release process
>>> (which I
>>>>>  agree was a lot of work)?
>>>>>>
>>>>>>   If your answer is 'it's still needed' then can we just
>>> unify
>>>>>  all other usages?
>>>>>>
>>>>>
>>>>>  Make a release is just the first of the problems. Take into account
>>>>>  that each release requires a vote and that vote takes 3 days to get
>>>>>  fixed. So in practice a problem in core can effectively block a
>>>>>  release of other artifacts. That's very inconvenient. Suppose we
>>> have
>>>>>  a new TCK and that one found a problem on myfaces core. Again even if
>>>>>  the other artifacts are good enough, this becomes a blocker. There are
>>>>>  enough historical evidence that supports this point. In conclusion
>>>>>  this slow down the whole release cycle we have on myfaces. So ignore
>>>>>  that is not an option.
>>>>>
>>>>>  Instead, maybe the option is reorganize myfaces core to allow
>>>>>  alternate release lifecycles per module. For example, each maven
>>>>>  plugin in myfaces has its own release lifecycle and there is a parent
>>>>>  pom with a different release procedure. This requires some changes to
>>>>>  create the source-release.zip file inherited from apache pom. But it
>>>>>  could be a cleaner solution.
>>>>>
>>>>>  This means myfaces-commons project should be "merged" in some
>>> way with
>>>>>  myfaces core. It has sense.
>>>>>
>>>>>>   One question which bothers me with the 'shared' approach
>>> if what
>>>>>  would happen to our build-tools annotation scanning
>>> (@JSFWebConfigParam, etc)?
>>>>>  Does this already work with dependencies? Do we have this problem
>>> already due to
>>>>>  the fact that we import such annotated classes via dependency?
>>>>>>
>>>>>
>>>>>  Those annotations comes from myfaces-builder-annotations. They are
>>>>>  source code annotations but all that information are saved on
>>>>>  myfaces-metadata.xml, so even if dissapear on compile time, the
>>>>>  information can be gathered from there. It is not a problem.
>>>>>
>>>>>>>   Additionally, we increase the risk of "side
>>> effects",
>>>>>>>   because a change done in core could introduce a bug in other
>>> parts.
>>>>>>   Imo it's exactly the opposite. If you use the same code in 7
>>> projects,
>>>>>  then it is more likely that a bug gets found and fixed.
>>>>>>   And the opposite case is (sadly) absolutely unlikely. If you have
>>> a class
>>>>>  duplicated 7 times and find a bug in one project, it is highly unlikely
>>> that all
>>>>>  6 other projects will get this fix applied :(
>>>>>>
>>>>>
>>>>>  But what happen when you have some code that does not have a clear
>>>>>  "interface". If somebody removes or change some code because
>>> he/she
>>>>>  thinks it is not used in core or whatever, all 6 projects that could
>>>>>  require it will be affected and will require to rework its code.
>>>>>  Things get uglier when you have one library working with version 1.1.1
>>>>>  and 1.1.2 is binary incompatible with version 1.1.1, but my other
>>>>>  dependency requires it and kaboooom, the application does not work.
>>>>>  So, the first assumption we need to preserve in those
>>> "shared"
>>>>>  artifacts is build it as an API, preserving binary compatibility.
>>>>>
>>>>>  So we can't just grab the code from shared as is and say to users
>>> "...
>>>>>  you can use that into its own projects ...". If the project is
>>>>>  maintained inside myfaces we can fix such problems, but outside
>>>>>  myfaces we should be more strict. So, we need a "public
>>> shared" code
>>>>>  like the one proposed in myfaces commons and other code "myfaces
>>>>>  shared" to use in projects like tomahawk or portletbridge or
>>> whatever
>>>>>  inside our land.
>>>>>
>>>>>  regards,
>>>>>
>>>>>  Leonardo Uribe
>>>>>
>>>>>>   LieGrue,
>>>>>>   strub
>>>>>>
>>>>>>
>>>>>>
>>>>>>   ----- Original Message -----
>>>>>>>   From: Leonardo Uribe <lu...@gmail.com>
>>>>>>>   To: MyFaces Development <de...@myfaces.apache.org>
>>>>>>>   Cc: Mark Struberg <st...@yahoo.de>
>>>>>>>   Sent: Sunday, October 23, 2011 9:08 PM
>>>>>>>   Subject: Re: [DISCUSS] how to get rid of tons of duplicated
>>> code
>>>>>>>
>>>>>>>   Hi
>>>>>>>
>>>>>>>   Ok, let's check the proposal
>>>>>>>
>>>>>>>   MS>> So the suggestion is:
>>>>>>>   MS>>
>>>>>>>   MS>> 1.) cleanup myfaces-shared. mf-shared has almost no
>>>>>  checkstyle
>>>>>>>   rules applied.
>>>>>>>
>>>>>>>   Yes, sounds good.
>>>>>>>
>>>>>>>   MS>> 2.) add unit tests for myfaces-shared. Currently
>>> there are
>>>>>  not
>>>>>>>   many...
>>>>>>>
>>>>>>>   Ok, sounds good too.
>>>>>>>
>>>>>>>   MS>> 3.) move the shared code parts back to
>>> myfaces-shared and
>>>>>  add unit
>>>>>>>   tests.
>>>>>>>
>>>>>>>   So, this means do one step back and move the code from
>>> myfaces-core
>>>>>>>   "shared" to myfaces-shared project? This breaks
>>> effectively
>>>>>  the
>>>>>>>   changes done some months ago to make easier work with myfaces
>>> core
>>>>>>>   itself.
>>>>>>>
>>>>>>>   In that time the conclusion was: "core has priority over
>>> anything
>>>>>>>   else, so shared code must live in core, but myfaces-shared
>>> project
>>>>>>>   should just copy the code from there and have its own
>>> lifecycle"
>>>>>>>   (these are my own words as I understood).
>>>>>>>
>>>>>>>   So this point does not have practical sense, and go against
>>> everything
>>>>>>>   discussed earlier.
>>>>>>>
>>>>>>>   MS>> 4.) import myfaces-shared via maven dependency and
>>> use
>>>>>>>   <minimizeJar> and <relocations> to package the
>>> stuff
>>>>>>>
>>>>>>>   maven-shade-plugin is a good "tool" but doesn't
>>> fit well
>>>>>  in this
>>>>>>>   scenario. The reason is we need an alternate release lifecycle
>>> for the
>>>>>>>   shared code between myfaces core and other projects.
>>>>>>>
>>>>>>>   Historically that was the very first intention behind
>>> myfaces-shared
>>>>>>>   project. Any myfaces core release requires some additional
>>> steps to do
>>>>>>>   (TCK), so that becomes a problem when you try to release other
>>>>>>>   libraries that depends of shared. So, to fix that,
>>> "shared"
>>>>>  was
>>>>>>>   created, so the code can be released in a independent way, and
>>> prevent
>>>>>>>   myfaces core becomes an obstacle to release any other project
>>>>>>>   (tomahawk, portlet-bridge, ... ). So, to release tomahawk you
>>> release
>>>>>>>   shared first and then tomahawk.
>>>>>>>
>>>>>>>   maven-shade-plugin requires a released artifact to do its job.
>>> So, use
>>>>>>>   it impose that restriction. In "shared" case,
>>> preserve the
>>>>>  original
>>>>>>>   intention becomes "imperative", and that's the
>>> reason why
>>>>>  a goal
>>>>>>>   was
>>>>>>>   created to copy the code from myfaces-core shared, so the
>>> release
>>>>>>>   manager can run this goal, commit the changes and then run a
>>> release.
>>>>>>>
>>>>>>>   My proposal in this case is do the same we did for shared, but
>>> for
>>>>>>>   "myfaces commons" case. Then we can use
>>> maven-shade-plugin in
>>>>>  other
>>>>>>>   projects, but not over shared, instead over a released version
>>> of
>>>>>>>   myfaces-commons-utils. Keep tomahawk or portlet-bridge as is,
>>> using
>>>>>>>   shared project, because by its nature, those projects require
>>> classes
>>>>>>>   that are not meant to be used outside those cases.
>>>>>>>
>>>>>>>   Note do any hack in this part makes a little bit
>>> "obscure"
>>>>>  how to make
>>>>>>>   changes, because everything becomes "centralized",
>>> but makes
>>>>>  easier
>>>>>>>   maintain code. Additionally, we increase the risk of
>>> "side
>>>>>  effects",
>>>>>>>   because a change done in core could introduce a bug in other
>>> parts. So
>>>>>>>   at the end this is a matter of how to keep our code
>>>>>  "balanced", even
>>>>>>>   if some times it becomes a decision about "choose the
>>> less
>>>>>>>   inconvenient alternative".
>>>>>>>
>>>>>>>   regards,
>>>>>>>
>>>>>>>   Leonardo Uribe
>>>>>>>
>>>>>>>   2011/10/23 Leonardo Uribe <lu...@gmail.com>:
>>>>>>>>    Hi
>>>>>>>>
>>>>>>>>    2011/10/23 Jakob Korherr <ja...@gmail.com>:
>>>>>>>>>    Hi Mark,
>>>>>>>>>
>>>>>>>>>    +1 - that's exactly what I have been trying to
>>> accomplish
>>>>>  some time
>>>>>>>>>    ago (introducing common-shades [1]). Unfortunately, I
>>> was not
>>>>>>>>>    successful back then.
>>>>>>>>>
>>>>>>>>
>>>>>>>>    It is clear we need to "split" myfaces-impl
>>> into
>>>>>  multiple
>>>>>>>   modules. There
>>>>>>>>    are some parts that are useful for other projects. The
>>> code you
>>>>>  did
>>>>>>>>    on commons-shade was the attempt to solve the problem of
>>> the
>>>>>>>>    duplicate code used on myfaces-test.
>>>>>>>>
>>>>>>>>    Now the objective is find a way about how to reuse code
>>> in myfaces
>>>>>>>>    core between multiple projects effectively.
>>>>>>>>
>>>>>>>>>    However, there is a slight problem with moving all
>>> this stuff
>>>>>  into
>>>>>>>>>    MyFaces shared, which I want to point out: code size.
>>> If we
>>>>>  really put
>>>>>>>>>    all the code that is shared across any MyFaces
>>> subproject into
>>>>>  shared,
>>>>>>>>>    it will get fat and ugly (even more than it is right
>>> now). In
>>>>>>>>>    addition, if we continue including the whole shared
>>> project
>>>>>  into
>>>>>>>>>    MyFaces core, MyFaces core impl will get bigger and
>>> bigger.
>>>>>>>>>
>>>>>>>>
>>>>>>>>    Yes, the problem basically is MyFaces shared does not
>>> have any
>>>>>  order
>>>>>>>>    or any notion of API. There are code that is used only in
>>> tomahawk
>>>>>  but not
>>>>>>>>    intended to use in any other place. There are some useful
>>>>>  utitlities but
>>>>>>>>    sometimes without documentation, and there are some other
>>> code
>>>>>  that is
>>>>>>>>    just obsolete. It it clear a cleanup of that location is
>>>>>>>>    necessary, but note priorities comes first, so this task
>>> has been
>>>>>  delayed
>>>>>>>   in
>>>>>>>>    order to deal with other important stuff. Now it is a
>>> good time to
>>>>>  fix
>>>>>>>   this.
>>>>>>>>
>>>>>>>>>    Thus I'd like to suggest something similar which
>>> I wanted
>>>>>  to
>>>>>>>>>    accomplish with common-shades: Introduce a new shared
>>> module,
>>>>>  which
>>>>>>>>>    consists of many submodules that each handle a
>>> specific
>>>>>  functionality
>>>>>>>>>    instead of being one fat module. With this approach
>>> each
>>>>>  MyFaces
>>>>>>>>>    subproject would be able to pick out only the stuff
>>> it really
>>>>>  needs.
>>>>>>>>>    Furthermore we would see more easily which code in
>>> shared is
>>>>>  not used
>>>>>>>>>    anymore (I guess at the moment there is a lot of it),
>>> just by
>>>>>  checking
>>>>>>>>>    which modules are still used in our poms.
>>>>>>>>>
>>>>>>>>
>>>>>>>>    That is the big question, how to split myfaces-impl and
>>> shared.
>>>>>  Precisely
>>>>>>>>    the intention of myfaces-commons-utils projects was take
>>> the stuff
>>>>>  that is
>>>>>>>>    useful from shared and build an usable API for developers
>>> outside
>>>>>  MyFaces.
>>>>>>>>
>>>>>>>>    For example, MyFaces HTML5 subproject was a good
>>> experiment to see
>>>>>>>>    which code is useful and should be added in a API. Some
>>> weeks ago
>>>>>  I checked
>>>>>>>>    and removed all duplicate code to use
>>> myfaces-commons-utils. So
>>>>>  the 1.0.2
>>>>>>>>    release contains those classes taken from shared.
>>>>>>>>
>>>>>>>>    regards,
>>>>>>>>
>>>>>>>>    Leonardo Uribe
>>>>>>>>
>>>>>>>>>    Regards,
>>>>>>>>>    Jakob
>>>>>>>>>
>>>>>>>>>    [1]
>>> https://svn.apache.org/repos/asf/myfaces/common-shades/
>>>>>>>>>
>>>>>>>>>    2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>>>>>>>>    Hi!
>>>>>>>>>>    While working on the mafyces-commons cleanup I
>>> figured
>>>>>  that we have
>>>>>>>   tons of
>>>>>>>>>>    duplicated code spread over MyFaces.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>    As an example I like to mention
>>>>>  myfaces-commons-resourcehandler.
>>>>>>>   There are
>>>>>>>>>>    43 classes in total, and 35 of them are just 1:1
>>> copied
>>>>>  from other
>>>>>>>   projects
>>>>>>>>>>    to provide resource management, zip, etc. For me
>>> this is
>>>>>  an
>>>>>>>   absolute no-go.
>>>>>>>>>>    Those classes have neither tests nor any
>>> documentation
>>>>>  where they
>>>>>>>   got forked
>>>>>>>>>>    from. Nor will any bug which gets fixed in
>>> another module
>>>>>  make
>>>>>>>   it's way over
>>>>>>>>>>    to all the other projects containing that very
>>> forked
>>>>>  code.
>>>>>>>   That's just ...
>>>>>>>>>>    unbelievable unmaintainable.
>>>>>>>>>>
>>>>>>>>>>    There are 2 different ways to solve this
>>> (depending on the
>>>>>>>   problem):
>>>>>>>>>>
>>>>>>>>>>    A.) drop the functionality and provide a
>>> generalized
>>>>>  solution. The
>>>>>>>   GZIP of
>>>>>>>>>>    myfaces-commons-resourcehandleris an obvious
>>> example:
>>>>>>>>>>    We now copy this code over the 4th time or even
>>> more.
>>>>>  Instead of
>>>>>>>   doing this,
>>>>>>>>>>    we should rather do it in the classic unix
>>> fashion: do one
>>>>>  thing,
>>>>>>>   but do it
>>>>>>>>>>    well.
>>>>>>>>>>    Which means I'd rather see all the GZIP stuff
>>> factored
>>>>>  out into
>>>>>>>   an own
>>>>>>>>>>    mf-commons module as a Servlet Filter. This can
>>> then get
>>>>>  applied to
>>>>>>>   what
>>>>>>>>>>    ever other mechanism you like. This could also
>>> (commonly)
>>>>>  cover
>>>>>>>   cases like
>>>>>>>>>>    detecting http UserAgents which are not able to
>>> handle
>>>>>  zipped
>>>>>>>   resources,
>>>>>>>>>>    etc. That way we could provide this logic ONCE
>>> and have
>>>>>  complete
>>>>>>>   freedom
>>>>>>>>>>    over the configuration.
>>>>>>>>>>
>>>>>>>>>>    B.) code reusable components once and use them in
>>> other
>>>>>  projects
>>>>>>>   (ev via
>>>>>>>>>>    shading it in).
>>>>>>>>>>    ClassLoaderResourceLoader would be a perfect
>>> candidate! I
>>>>>  grepped
>>>>>>>   through
>>>>>>>>>>    only the few pits which I have checked out
>>> locally and
>>>>>  found this
>>>>>>>   class 7
>>>>>>>>>>    SEVEN times! I just can't believe that we
>>> can't
>>>>>  move this
>>>>>>>   stuff to a shared
>>>>>>>>>>    modul...
>>>>>>>>>>
>>>>>>>>>>    Same for FacesServletMapping. 6 times copied
>>> around,
>>>>>>>>>>    WebConfigProviderFactory 5 times, ...
>>>>>>>>>>    There are whole packages with 10++ classes which
>>> got
>>>>>  copied 1:1!
>>>>>>>>>>
>>>>>>>>>>    I really could cry seeing this :(
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>    What can we do to solve this?
>>>>>>>>>>
>>>>>>>>>>    Theoretically myfaces-shared should contain this
>>> stuff.
>>>>>  This is
>>>>>>>   exactly what
>>>>>>>>>>    it is for!
>>>>>>>>>>    Historically there have been some hand forged
>>> tweeks and
>>>>>  ugly
>>>>>>>   hacks, but
>>>>>>>>>>    nowadays we have the maven-shade-plugin to make
>>> our live
>>>>>  easier.
>>>>>>>>>>
>>>>>>>>>>    So the suggestion is:
>>>>>>>>>>
>>>>>>>>>>    1.) cleanup myfaces-shared. mf-shared has almost
>>> no
>>>>>  checkstyle
>>>>>>>   rules
>>>>>>>>>>    applied.
>>>>>>>>>>    2.) add unit tests for myfaces-shared. Currently
>>> there are
>>>>>  not
>>>>>>>   many...
>>>>>>>>>>    3.) move the shared code parts back to
>>> myfaces-shared and
>>>>>  add unit
>>>>>>>   tests.
>>>>>>>>>>    4.) import myfaces-shared via maven dependency
>>> and use
>>>>>>>   <minimizeJar> and
>>>>>>>>>>    <relocations> to package the stuff
>>>>>>>>>>
>>>>>>>>>>    [+1] fine go ahead (ideally: yes, what parts can
>>> I help
>>>>>  with?)
>>>>>>>>>>    [0] dont care
>>>>>>>>>>    [-1] wont work because ...
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>    I've attached a file which contains all
>>> Classes which
>>>>>  name
>>>>>>>   exists multiple
>>>>>>>>>>    times in MyFaces. The number is the cound how
>>> often they
>>>>>  exist in
>>>>>>>   MyFaces. I
>>>>>>>>>>    excluded current20.
>>>>>>>>>>    Please note that classes with the same name do
>>> not
>>>>>  necessarily have
>>>>>>>   the same
>>>>>>>>>>    content - but quite a lot actually do have!
>>> (scroll to the
>>>>>  bottom
>>>>>>>   of the
>>>>>>>>>>    file ...)
>>>>>>>>>>
>>>>>>>>>>    LieGrue,
>>>>>>>>>>    strub
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>    --
>>>>>>>>>    Jakob Korherr
>>>>>>>>>
>>>>>>>>>    blog: http://www.jakobk.com
>>>>>>>>>    twitter: http://twitter.com/jakobkorherr
>>>>>>>>>    work: http://www.irian.at
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: [DISCUSS] how to get rid of tons of duplicated code

[Leonardo Uribe <lu...@gmail.com>]

Hi

I started a branch to try it:

http://svn.apache.org/repos/asf/myfaces/core/branches/2.0.x_refactor_shade_duplicate/

just to gain some time. I think it is worth at least to try a prototype.

regards,

Leonardo Uribe

2011/10/24 Mark Struberg <st...@yahoo.de>:
> Hi Leo!
>
> Yes, this sounds much better, but please give me 2 days to think through it in detail.
>
> txs and LieGrue,
> strub
>
>
>
> ----- Original Message -----
>> From: Leonardo Uribe <lu...@gmail.com>
>> To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg <st...@yahoo.de>
>> Cc:
>> Sent: Monday, October 24, 2011 7:18 PM
>> Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>
>> Hi
>>
>> Ok, it is evident we have different opinions here about how to deal
>> with this problem. So, rather than refute the arguments I think it is
>> more productive to show another proposal. In MyFaces Core 2.0.x we
>> have the following layout:
>>
>> api/
>> assembly/
>> bundle/
>> impl/
>> implee6/
>> integration-tests/
>> pom.xml
>> shared/
>> src/
>>
>> Let's add two modules
>>
>> parent : contains the parent POM that all submodules should inherit.
>> shared-utils : utilities for JSF used in core, but built as an api.
>>
>> Now, we move the duplicate code related to myfaces-commons-utils into
>> this module, and shared will have shared-utils as dependency. impl
>> module will use maven-shade plugin to take the code from shared-utils
>> and shared (actually this is done for only for shared).
>>
>> When a release of myfaces-core is done, all modules are released, so
>> things go as usual. But have the parent as module has the advantage
>> that if we want to release shared-utils or shared internal modules, we
>> can do it only releasing parent (optional) and shared-utils.
>>
>> Since we can create a release for these modules, we remove the hack
>> used on on shared project (hard copy). Just we modify the pom to use
>> maven-shade-plugin over myfaces-core shared module. Then we kill
>> shared-tomahawk, and we make tomahawk use maven-shade-plugin over
>> shaded artifact in shared project.
>>
>> In commons we do the same as in shared. In
>> myfaces-commons-resourcehandler we use myfaces core shared module
>> using maven-shade-plugin.
>>
>> The only disadvantage is the release process gets a little bit more
>> complicated, but since do a release becomes easier with nexus, it is
>> ok.
>>
>> Do you agree with this solution?
>>
>> regards,
>>
>> Leonardo Uribe
>>
>> 2011/10/24 Mark Struberg <st...@yahoo.de>:
>>>> Really that was not solved using maven-shade-plugin.
>>>  Then we used the maven-shade-plugin the wrong way. See the
>> <relocations> option [1].
>>>
>>>>  There, there is a profile called
>>>>  "synch-myfaces-impl-shared", when it is added, the code is
>> copied and
>>>>  then a manual commit do the trick.
>>>
>>>  I think this is an ugly hack and doesn't solve any problems because
>>>
>>>  a.)
>>>>  Take into account
>>>>  that each release requires a vote and that vote takes 3 days to get
>>>>  fixed.
>>>  you could just do a mvn release of shared + core in 1 go to the same
>> staging repo -> only 1 vote is needed!
>>>  This argument is simply wrong.
>>>
>>>  b.)
>>>  You
>>>   currently copy the code over 1:1 (half manually) thus your argument
>>>  with 'core and other projects need different sources' is just nil.
>> There
>>>   is no difference if you do this by profile, by hand or automatically!
>>>  So I really prefer to have this automatically. Which is exactly what a
>>>  dependency does...
>>>
>>>  c.) the TCK argument is moot because it has
>>>  nothing to do with shared. Most of the issues in the TCK are not
>>>  affecting shared. And if they do, then those fixes are needed BY ALL
>>>  OTHER PROJECTS AS WELL. Thus another argument against hiding this code
>>>  and duplicating it all over...
>>>
>>>  c.)
>>>>  Instead, maybe the option is reorganize myfaces core to allow
>>>>  alternate release lifecycles per module
>>>  Sorry I don't grok this argument. It sounds as it would make all things
>> more complicated without solving any real problem.
>>>
>>>  e.)
>>>>  This means myfaces-commons project should be "merged" in some
>> way with
>>>>  myfaces core. It has sense.
>>>  2 options:
>>>  1..) kill myfaces-shared completely and use the shared from core instead.
>>>  Downside: if you need some fix in the shared code for some other project,
>> you would need to release mf-core
>>>  2.) kill the newly introduced (this got only created a few weeks back by
>> you) core-shared and use mf-shared again.
>>>  Downside: hmmm nothing if one understands how to release correctly.
>>>
>>>  f.)
>>>   all your explanations only explain the duplication between
>>>  myfaces-shared and myfaces-core-shared. I can live with the duplication
>>>  for now, but we also have classes which got copied around up to 8 times!
>>>   There is no excuse for that imo.
>>>
>>>  g.)
>>>>  But what happen when you have some code that does not have a clear
>>>>  "interface". If somebody removes or change some code because
>> he/she
>>>>  thinks it is not used in core or whatever, all 6 projects that could
>>>>  require it will be affected and will require to rework its code.
>>>>  Things get uglier when you have one library working with version 1.1.1
>>>>  and 1.1.2 is binary incompatible with version 1.1.1, but my other
>>>>  dependency requires it and kaboooom, the application does not work.
>>>>  So, the first assumption we need to preserve in those
>> "shared"
>>>>  artifacts is build it as an API, preserving binary compatibility.
>>>
>>>  I don't get that argument neither!
>>>  Hey,
>>>   that's life! If it turns out that the code is not good enough and
>> needs
>>>   a fix, then that's the way it is! All other projects should fix that
>>>  too in that case. I rather have a reproducible compile error which
>>>  easily could get fixed than having tons of duplicated code which is more
>>>   or less always logically broken and badly tested.
>>>  Yes, we should be
>>>  aware that the classes we put into myfaces-shared must meet some
>>>  standards and need to be well tested. But actually that would benefit
>>>  our project a lot.
>>>
>>>
>>>  h.) I just realised that our process in copying shared-impl from core to
>> mf-shared is even more broken than every process before.
>>>  If you are working on a lets say mf-commons project and find a bug in any
>> of those shared parts, then you would need to RELEASE MF-CORE FIRST? omg, this
>> cannot be serious!
>>>
>>>
>>>  LieGrue,
>>>  strub
>>>
>>>
>>>  [1]
>> http://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#relocations
>>>
>>>
>>>
>>>  ----- Original Message -----
>>>>  From: Leonardo Uribe <lu...@gmail.com>
>>>>  To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg
>> <st...@yahoo.de>
>>>>  Cc:
>>>>  Sent: Monday, October 24, 2011 4:36 AM
>>>>  Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>>>
>>>>  Hi
>>>>
>>>>  2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>>>   I've now read through the old mail archives and understand
>> what the
>>>>  original problem was. But actually I don't think we solved it
>> correctly
>>>>  right now. Of course we solved to original problem, but opened a can of
>> worms
>>>>  causing other problems.
>>>>>
>>>>>   The problem as far as I remember has been that myfaces-shared had
>> tons of
>>>>  duplicated code in it. One for core, one for tomahawk, one for
>> trinidad, etc.
>>>>>
>>>>>
>>>>>   The shared part for core got moved to myfaces-core, but the deeper
>> problem
>>>>  was that it was not easily possible to have multiple different versions
>> of
>>>>  myfaces-shared. This now got solved by using the maven-shade-plugin. So
>> we
>>>>  should rethink the practice to duplicate all the code and aim for a
>> _clean_
>>>>  solution.
>>>>>
>>>>
>>>>  Really that was not solved using maven-shade-plugin. What we did was
>>>>  copy the code into myfaces-core and create a mirror of the same code
>>>>  under shared. There, there is a profile called
>>>>  "synch-myfaces-impl-shared", when it is added, the code is
>> copied and
>>>>  then a manual commit do the trick.
>>>>
>>>>>   Also (being a maven guy) I cannot quite follow the argument about
>> the
>>>>  release cycles. Running a myfaces-shared release and then (with the
>> same staging
>>>>  repo) a myfaces-core release is a task of 15 minutes. + the time for
>> running the
>>>>  TCK, but this gets run via CI anyway, right? Thus this is barely a
>> problem.
>>>>>   If it is then I'd happily volunteer to do the next release (do
>> this for
>>>>  a few projects already) As you know, performing a release really got
>> _much_
>>>>  easier nowadays with our new apache-parent pom.
>>>>>   But maybe this argument was only meant for our old release process
>> (which I
>>>>  agree was a lot of work)?
>>>>>
>>>>>   If your answer is 'it's still needed' then can we just
>> unify
>>>>  all other usages?
>>>>>
>>>>
>>>>  Make a release is just the first of the problems. Take into account
>>>>  that each release requires a vote and that vote takes 3 days to get
>>>>  fixed. So in practice a problem in core can effectively block a
>>>>  release of other artifacts. That's very inconvenient. Suppose we
>> have
>>>>  a new TCK and that one found a problem on myfaces core. Again even if
>>>>  the other artifacts are good enough, this becomes a blocker. There are
>>>>  enough historical evidence that supports this point. In conclusion
>>>>  this slow down the whole release cycle we have on myfaces. So ignore
>>>>  that is not an option.
>>>>
>>>>  Instead, maybe the option is reorganize myfaces core to allow
>>>>  alternate release lifecycles per module. For example, each maven
>>>>  plugin in myfaces has its own release lifecycle and there is a parent
>>>>  pom with a different release procedure. This requires some changes to
>>>>  create the source-release.zip file inherited from apache pom. But it
>>>>  could be a cleaner solution.
>>>>
>>>>  This means myfaces-commons project should be "merged" in some
>> way with
>>>>  myfaces core. It has sense.
>>>>
>>>>>   One question which bothers me with the 'shared' approach
>> if what
>>>>  would happen to our build-tools annotation scanning
>> (@JSFWebConfigParam, etc)?
>>>>  Does this already work with dependencies? Do we have this problem
>> already due to
>>>>  the fact that we import such annotated classes via dependency?
>>>>>
>>>>
>>>>  Those annotations comes from myfaces-builder-annotations. They are
>>>>  source code annotations but all that information are saved on
>>>>  myfaces-metadata.xml, so even if dissapear on compile time, the
>>>>  information can be gathered from there. It is not a problem.
>>>>
>>>>>>   Additionally, we increase the risk of "side
>> effects",
>>>>>>   because a change done in core could introduce a bug in other
>> parts.
>>>>>   Imo it's exactly the opposite. If you use the same code in 7
>> projects,
>>>>  then it is more likely that a bug gets found and fixed.
>>>>>   And the opposite case is (sadly) absolutely unlikely. If you have
>> a class
>>>>  duplicated 7 times and find a bug in one project, it is highly unlikely
>> that all
>>>>  6 other projects will get this fix applied :(
>>>>>
>>>>
>>>>  But what happen when you have some code that does not have a clear
>>>>  "interface". If somebody removes or change some code because
>> he/she
>>>>  thinks it is not used in core or whatever, all 6 projects that could
>>>>  require it will be affected and will require to rework its code.
>>>>  Things get uglier when you have one library working with version 1.1.1
>>>>  and 1.1.2 is binary incompatible with version 1.1.1, but my other
>>>>  dependency requires it and kaboooom, the application does not work.
>>>>  So, the first assumption we need to preserve in those
>> "shared"
>>>>  artifacts is build it as an API, preserving binary compatibility.
>>>>
>>>>  So we can't just grab the code from shared as is and say to users
>> "...
>>>>  you can use that into its own projects ...". If the project is
>>>>  maintained inside myfaces we can fix such problems, but outside
>>>>  myfaces we should be more strict. So, we need a "public
>> shared" code
>>>>  like the one proposed in myfaces commons and other code "myfaces
>>>>  shared" to use in projects like tomahawk or portletbridge or
>> whatever
>>>>  inside our land.
>>>>
>>>>  regards,
>>>>
>>>>  Leonardo Uribe
>>>>
>>>>>   LieGrue,
>>>>>   strub
>>>>>
>>>>>
>>>>>
>>>>>   ----- Original Message -----
>>>>>>   From: Leonardo Uribe <lu...@gmail.com>
>>>>>>   To: MyFaces Development <de...@myfaces.apache.org>
>>>>>>   Cc: Mark Struberg <st...@yahoo.de>
>>>>>>   Sent: Sunday, October 23, 2011 9:08 PM
>>>>>>   Subject: Re: [DISCUSS] how to get rid of tons of duplicated
>> code
>>>>>>
>>>>>>   Hi
>>>>>>
>>>>>>   Ok, let's check the proposal
>>>>>>
>>>>>>   MS>> So the suggestion is:
>>>>>>   MS>>
>>>>>>   MS>> 1.) cleanup myfaces-shared. mf-shared has almost no
>>>>  checkstyle
>>>>>>   rules applied.
>>>>>>
>>>>>>   Yes, sounds good.
>>>>>>
>>>>>>   MS>> 2.) add unit tests for myfaces-shared. Currently
>> there are
>>>>  not
>>>>>>   many...
>>>>>>
>>>>>>   Ok, sounds good too.
>>>>>>
>>>>>>   MS>> 3.) move the shared code parts back to
>> myfaces-shared and
>>>>  add unit
>>>>>>   tests.
>>>>>>
>>>>>>   So, this means do one step back and move the code from
>> myfaces-core
>>>>>>   "shared" to myfaces-shared project? This breaks
>> effectively
>>>>  the
>>>>>>   changes done some months ago to make easier work with myfaces
>> core
>>>>>>   itself.
>>>>>>
>>>>>>   In that time the conclusion was: "core has priority over
>> anything
>>>>>>   else, so shared code must live in core, but myfaces-shared
>> project
>>>>>>   should just copy the code from there and have its own
>> lifecycle"
>>>>>>   (these are my own words as I understood).
>>>>>>
>>>>>>   So this point does not have practical sense, and go against
>> everything
>>>>>>   discussed earlier.
>>>>>>
>>>>>>   MS>> 4.) import myfaces-shared via maven dependency and
>> use
>>>>>>   <minimizeJar> and <relocations> to package the
>> stuff
>>>>>>
>>>>>>   maven-shade-plugin is a good "tool" but doesn't
>> fit well
>>>>  in this
>>>>>>   scenario. The reason is we need an alternate release lifecycle
>> for the
>>>>>>   shared code between myfaces core and other projects.
>>>>>>
>>>>>>   Historically that was the very first intention behind
>> myfaces-shared
>>>>>>   project. Any myfaces core release requires some additional
>> steps to do
>>>>>>   (TCK), so that becomes a problem when you try to release other
>>>>>>   libraries that depends of shared. So, to fix that,
>> "shared"
>>>>  was
>>>>>>   created, so the code can be released in a independent way, and
>> prevent
>>>>>>   myfaces core becomes an obstacle to release any other project
>>>>>>   (tomahawk, portlet-bridge, ... ). So, to release tomahawk you
>> release
>>>>>>   shared first and then tomahawk.
>>>>>>
>>>>>>   maven-shade-plugin requires a released artifact to do its job.
>> So, use
>>>>>>   it impose that restriction. In "shared" case,
>> preserve the
>>>>  original
>>>>>>   intention becomes "imperative", and that's the
>> reason why
>>>>  a goal
>>>>>>   was
>>>>>>   created to copy the code from myfaces-core shared, so the
>> release
>>>>>>   manager can run this goal, commit the changes and then run a
>> release.
>>>>>>
>>>>>>   My proposal in this case is do the same we did for shared, but
>> for
>>>>>>   "myfaces commons" case. Then we can use
>> maven-shade-plugin in
>>>>  other
>>>>>>   projects, but not over shared, instead over a released version
>> of
>>>>>>   myfaces-commons-utils. Keep tomahawk or portlet-bridge as is,
>> using
>>>>>>   shared project, because by its nature, those projects require
>> classes
>>>>>>   that are not meant to be used outside those cases.
>>>>>>
>>>>>>   Note do any hack in this part makes a little bit
>> "obscure"
>>>>  how to make
>>>>>>   changes, because everything becomes "centralized",
>> but makes
>>>>  easier
>>>>>>   maintain code. Additionally, we increase the risk of
>> "side
>>>>  effects",
>>>>>>   because a change done in core could introduce a bug in other
>> parts. So
>>>>>>   at the end this is a matter of how to keep our code
>>>>  "balanced", even
>>>>>>   if some times it becomes a decision about "choose the
>> less
>>>>>>   inconvenient alternative".
>>>>>>
>>>>>>   regards,
>>>>>>
>>>>>>   Leonardo Uribe
>>>>>>
>>>>>>   2011/10/23 Leonardo Uribe <lu...@gmail.com>:
>>>>>>>    Hi
>>>>>>>
>>>>>>>    2011/10/23 Jakob Korherr <ja...@gmail.com>:
>>>>>>>>    Hi Mark,
>>>>>>>>
>>>>>>>>    +1 - that's exactly what I have been trying to
>> accomplish
>>>>  some time
>>>>>>>>    ago (introducing common-shades [1]). Unfortunately, I
>> was not
>>>>>>>>    successful back then.
>>>>>>>>
>>>>>>>
>>>>>>>    It is clear we need to "split" myfaces-impl
>> into
>>>>  multiple
>>>>>>   modules. There
>>>>>>>    are some parts that are useful for other projects. The
>> code you
>>>>  did
>>>>>>>    on commons-shade was the attempt to solve the problem of
>> the
>>>>>>>    duplicate code used on myfaces-test.
>>>>>>>
>>>>>>>    Now the objective is find a way about how to reuse code
>> in myfaces
>>>>>>>    core between multiple projects effectively.
>>>>>>>
>>>>>>>>    However, there is a slight problem with moving all
>> this stuff
>>>>  into
>>>>>>>>    MyFaces shared, which I want to point out: code size.
>> If we
>>>>  really put
>>>>>>>>    all the code that is shared across any MyFaces
>> subproject into
>>>>  shared,
>>>>>>>>    it will get fat and ugly (even more than it is right
>> now). In
>>>>>>>>    addition, if we continue including the whole shared
>> project
>>>>  into
>>>>>>>>    MyFaces core, MyFaces core impl will get bigger and
>> bigger.
>>>>>>>>
>>>>>>>
>>>>>>>    Yes, the problem basically is MyFaces shared does not
>> have any
>>>>  order
>>>>>>>    or any notion of API. There are code that is used only in
>> tomahawk
>>>>  but not
>>>>>>>    intended to use in any other place. There are some useful
>>>>  utitlities but
>>>>>>>    sometimes without documentation, and there are some other
>> code
>>>>  that is
>>>>>>>    just obsolete. It it clear a cleanup of that location is
>>>>>>>    necessary, but note priorities comes first, so this task
>> has been
>>>>  delayed
>>>>>>   in
>>>>>>>    order to deal with other important stuff. Now it is a
>> good time to
>>>>  fix
>>>>>>   this.
>>>>>>>
>>>>>>>>    Thus I'd like to suggest something similar which
>> I wanted
>>>>  to
>>>>>>>>    accomplish with common-shades: Introduce a new shared
>> module,
>>>>  which
>>>>>>>>    consists of many submodules that each handle a
>> specific
>>>>  functionality
>>>>>>>>    instead of being one fat module. With this approach
>> each
>>>>  MyFaces
>>>>>>>>    subproject would be able to pick out only the stuff
>> it really
>>>>  needs.
>>>>>>>>    Furthermore we would see more easily which code in
>> shared is
>>>>  not used
>>>>>>>>    anymore (I guess at the moment there is a lot of it),
>> just by
>>>>  checking
>>>>>>>>    which modules are still used in our poms.
>>>>>>>>
>>>>>>>
>>>>>>>    That is the big question, how to split myfaces-impl and
>> shared.
>>>>  Precisely
>>>>>>>    the intention of myfaces-commons-utils projects was take
>> the stuff
>>>>  that is
>>>>>>>    useful from shared and build an usable API for developers
>> outside
>>>>  MyFaces.
>>>>>>>
>>>>>>>    For example, MyFaces HTML5 subproject was a good
>> experiment to see
>>>>>>>    which code is useful and should be added in a API. Some
>> weeks ago
>>>>  I checked
>>>>>>>    and removed all duplicate code to use
>> myfaces-commons-utils. So
>>>>  the 1.0.2
>>>>>>>    release contains those classes taken from shared.
>>>>>>>
>>>>>>>    regards,
>>>>>>>
>>>>>>>    Leonardo Uribe
>>>>>>>
>>>>>>>>    Regards,
>>>>>>>>    Jakob
>>>>>>>>
>>>>>>>>    [1]
>> https://svn.apache.org/repos/asf/myfaces/common-shades/
>>>>>>>>
>>>>>>>>    2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>>>>>>>    Hi!
>>>>>>>>>    While working on the mafyces-commons cleanup I
>> figured
>>>>  that we have
>>>>>>   tons of
>>>>>>>>>    duplicated code spread over MyFaces.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>    As an example I like to mention
>>>>  myfaces-commons-resourcehandler.
>>>>>>   There are
>>>>>>>>>    43 classes in total, and 35 of them are just 1:1
>> copied
>>>>  from other
>>>>>>   projects
>>>>>>>>>    to provide resource management, zip, etc. For me
>> this is
>>>>  an
>>>>>>   absolute no-go.
>>>>>>>>>    Those classes have neither tests nor any
>> documentation
>>>>  where they
>>>>>>   got forked
>>>>>>>>>    from. Nor will any bug which gets fixed in
>> another module
>>>>  make
>>>>>>   it's way over
>>>>>>>>>    to all the other projects containing that very
>> forked
>>>>  code.
>>>>>>   That's just ...
>>>>>>>>>    unbelievable unmaintainable.
>>>>>>>>>
>>>>>>>>>    There are 2 different ways to solve this
>> (depending on the
>>>>>>   problem):
>>>>>>>>>
>>>>>>>>>    A.) drop the functionality and provide a
>> generalized
>>>>  solution. The
>>>>>>   GZIP of
>>>>>>>>>    myfaces-commons-resourcehandleris an obvious
>> example:
>>>>>>>>>    We now copy this code over the 4th time or even
>> more.
>>>>  Instead of
>>>>>>   doing this,
>>>>>>>>>    we should rather do it in the classic unix
>> fashion: do one
>>>>  thing,
>>>>>>   but do it
>>>>>>>>>    well.
>>>>>>>>>    Which means I'd rather see all the GZIP stuff
>> factored
>>>>  out into
>>>>>>   an own
>>>>>>>>>    mf-commons module as a Servlet Filter. This can
>> then get
>>>>  applied to
>>>>>>   what
>>>>>>>>>    ever other mechanism you like. This could also
>> (commonly)
>>>>  cover
>>>>>>   cases like
>>>>>>>>>    detecting http UserAgents which are not able to
>> handle
>>>>  zipped
>>>>>>   resources,
>>>>>>>>>    etc. That way we could provide this logic ONCE
>> and have
>>>>  complete
>>>>>>   freedom
>>>>>>>>>    over the configuration.
>>>>>>>>>
>>>>>>>>>    B.) code reusable components once and use them in
>> other
>>>>  projects
>>>>>>   (ev via
>>>>>>>>>    shading it in).
>>>>>>>>>    ClassLoaderResourceLoader would be a perfect
>> candidate! I
>>>>  grepped
>>>>>>   through
>>>>>>>>>    only the few pits which I have checked out
>> locally and
>>>>  found this
>>>>>>   class 7
>>>>>>>>>    SEVEN times! I just can't believe that we
>> can't
>>>>  move this
>>>>>>   stuff to a shared
>>>>>>>>>    modul...
>>>>>>>>>
>>>>>>>>>    Same for FacesServletMapping. 6 times copied
>> around,
>>>>>>>>>    WebConfigProviderFactory 5 times, ...
>>>>>>>>>    There are whole packages with 10++ classes which
>> got
>>>>  copied 1:1!
>>>>>>>>>
>>>>>>>>>    I really could cry seeing this :(
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>    What can we do to solve this?
>>>>>>>>>
>>>>>>>>>    Theoretically myfaces-shared should contain this
>> stuff.
>>>>  This is
>>>>>>   exactly what
>>>>>>>>>    it is for!
>>>>>>>>>    Historically there have been some hand forged
>> tweeks and
>>>>  ugly
>>>>>>   hacks, but
>>>>>>>>>    nowadays we have the maven-shade-plugin to make
>> our live
>>>>  easier.
>>>>>>>>>
>>>>>>>>>    So the suggestion is:
>>>>>>>>>
>>>>>>>>>    1.) cleanup myfaces-shared. mf-shared has almost
>> no
>>>>  checkstyle
>>>>>>   rules
>>>>>>>>>    applied.
>>>>>>>>>    2.) add unit tests for myfaces-shared. Currently
>> there are
>>>>  not
>>>>>>   many...
>>>>>>>>>    3.) move the shared code parts back to
>> myfaces-shared and
>>>>  add unit
>>>>>>   tests.
>>>>>>>>>    4.) import myfaces-shared via maven dependency
>> and use
>>>>>>   <minimizeJar> and
>>>>>>>>>    <relocations> to package the stuff
>>>>>>>>>
>>>>>>>>>    [+1] fine go ahead (ideally: yes, what parts can
>> I help
>>>>  with?)
>>>>>>>>>    [0] dont care
>>>>>>>>>    [-1] wont work because ...
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>    I've attached a file which contains all
>> Classes which
>>>>  name
>>>>>>   exists multiple
>>>>>>>>>    times in MyFaces. The number is the cound how
>> often they
>>>>  exist in
>>>>>>   MyFaces. I
>>>>>>>>>    excluded current20.
>>>>>>>>>    Please note that classes with the same name do
>> not
>>>>  necessarily have
>>>>>>   the same
>>>>>>>>>    content - but quite a lot actually do have!
>> (scroll to the
>>>>  bottom
>>>>>>   of the
>>>>>>>>>    file ...)
>>>>>>>>>
>>>>>>>>>    LieGrue,
>>>>>>>>>    strub
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>    --
>>>>>>>>    Jakob Korherr
>>>>>>>>
>>>>>>>>    blog: http://www.jakobk.com
>>>>>>>>    twitter: http://twitter.com/jakobkorherr
>>>>>>>>    work: http://www.irian.at
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: [DISCUSS] how to get rid of tons of duplicated code

[Mark Struberg <st...@yahoo.de>]

Hi Leo!

Yes, this sounds much better, but please give me 2 days to think through it in detail.

txs and LieGrue,
strub



----- Original Message -----
> From: Leonardo Uribe <lu...@gmail.com>
> To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg <st...@yahoo.de>
> Cc: 
> Sent: Monday, October 24, 2011 7:18 PM
> Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
> 
> Hi
> 
> Ok, it is evident we have different opinions here about how to deal
> with this problem. So, rather than refute the arguments I think it is
> more productive to show another proposal. In MyFaces Core 2.0.x we
> have the following layout:
> 
> api/
> assembly/
> bundle/
> impl/
> implee6/
> integration-tests/
> pom.xml
> shared/
> src/
> 
> Let's add two modules
> 
> parent : contains the parent POM that all submodules should inherit.
> shared-utils : utilities for JSF used in core, but built as an api.
> 
> Now, we move the duplicate code related to myfaces-commons-utils into
> this module, and shared will have shared-utils as dependency. impl
> module will use maven-shade plugin to take the code from shared-utils
> and shared (actually this is done for only for shared).
> 
> When a release of myfaces-core is done, all modules are released, so
> things go as usual. But have the parent as module has the advantage
> that if we want to release shared-utils or shared internal modules, we
> can do it only releasing parent (optional) and shared-utils.
> 
> Since we can create a release for these modules, we remove the hack
> used on on shared project (hard copy). Just we modify the pom to use
> maven-shade-plugin over myfaces-core shared module. Then we kill
> shared-tomahawk, and we make tomahawk use maven-shade-plugin over
> shaded artifact in shared project.
> 
> In commons we do the same as in shared. In
> myfaces-commons-resourcehandler we use myfaces core shared module
> using maven-shade-plugin.
> 
> The only disadvantage is the release process gets a little bit more
> complicated, but since do a release becomes easier with nexus, it is
> ok.
> 
> Do you agree with this solution?
> 
> regards,
> 
> Leonardo Uribe
> 
> 2011/10/24 Mark Struberg <st...@yahoo.de>:
>>> Really that was not solved using maven-shade-plugin.
>>  Then we used the maven-shade-plugin the wrong way. See the 
> <relocations> option [1].
>> 
>>>  There, there is a profile called
>>>  "synch-myfaces-impl-shared", when it is added, the code is 
> copied and
>>>  then a manual commit do the trick.
>> 
>>  I think this is an ugly hack and doesn't solve any problems because
>> 
>>  a.)
>>>  Take into account
>>>  that each release requires a vote and that vote takes 3 days to get
>>>  fixed.
>>  you could just do a mvn release of shared + core in 1 go to the same 
> staging repo -> only 1 vote is needed!
>>  This argument is simply wrong.
>> 
>>  b.)
>>  You
>>   currently copy the code over 1:1 (half manually) thus your argument
>>  with 'core and other projects need different sources' is just nil. 
> There
>>   is no difference if you do this by profile, by hand or automatically!
>>  So I really prefer to have this automatically. Which is exactly what a
>>  dependency does...
>> 
>>  c.) the TCK argument is moot because it has
>>  nothing to do with shared. Most of the issues in the TCK are not
>>  affecting shared. And if they do, then those fixes are needed BY ALL
>>  OTHER PROJECTS AS WELL. Thus another argument against hiding this code
>>  and duplicating it all over...
>> 
>>  c.)
>>>  Instead, maybe the option is reorganize myfaces core to allow
>>>  alternate release lifecycles per module
>>  Sorry I don't grok this argument. It sounds as it would make all things 
> more complicated without solving any real problem.
>> 
>>  e.)
>>>  This means myfaces-commons project should be "merged" in some 
> way with
>>>  myfaces core. It has sense.
>>  2 options:
>>  1..) kill myfaces-shared completely and use the shared from core instead.
>>  Downside: if you need some fix in the shared code for some other project, 
> you would need to release mf-core
>>  2.) kill the newly introduced (this got only created a few weeks back by 
> you) core-shared and use mf-shared again.
>>  Downside: hmmm nothing if one understands how to release correctly.
>> 
>>  f.)
>>   all your explanations only explain the duplication between
>>  myfaces-shared and myfaces-core-shared. I can live with the duplication
>>  for now, but we also have classes which got copied around up to 8 times!
>>   There is no excuse for that imo.
>> 
>>  g.)
>>>  But what happen when you have some code that does not have a clear
>>>  "interface". If somebody removes or change some code because 
> he/she
>>>  thinks it is not used in core or whatever, all 6 projects that could
>>>  require it will be affected and will require to rework its code.
>>>  Things get uglier when you have one library working with version 1.1.1
>>>  and 1.1.2 is binary incompatible with version 1.1.1, but my other
>>>  dependency requires it and kaboooom, the application does not work.
>>>  So, the first assumption we need to preserve in those 
> "shared"
>>>  artifacts is build it as an API, preserving binary compatibility.
>> 
>>  I don't get that argument neither!
>>  Hey,
>>   that's life! If it turns out that the code is not good enough and 
> needs
>>   a fix, then that's the way it is! All other projects should fix that
>>  too in that case. I rather have a reproducible compile error which
>>  easily could get fixed than having tons of duplicated code which is more
>>   or less always logically broken and badly tested.
>>  Yes, we should be
>>  aware that the classes we put into myfaces-shared must meet some
>>  standards and need to be well tested. But actually that would benefit
>>  our project a lot.
>> 
>> 
>>  h.) I just realised that our process in copying shared-impl from core to 
> mf-shared is even more broken than every process before.
>>  If you are working on a lets say mf-commons project and find a bug in any 
> of those shared parts, then you would need to RELEASE MF-CORE FIRST? omg, this 
> cannot be serious!
>> 
>> 
>>  LieGrue,
>>  strub
>> 
>> 
>>  [1] 
> http://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#relocations
>> 
>> 
>> 
>>  ----- Original Message -----
>>>  From: Leonardo Uribe <lu...@gmail.com>
>>>  To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg 
> <st...@yahoo.de>
>>>  Cc:
>>>  Sent: Monday, October 24, 2011 4:36 AM
>>>  Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>> 
>>>  Hi
>>> 
>>>  2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>>   I've now read through the old mail archives and understand 
> what the
>>>  original problem was. But actually I don't think we solved it 
> correctly
>>>  right now. Of course we solved to original problem, but opened a can of 
> worms
>>>  causing other problems.
>>>> 
>>>>   The problem as far as I remember has been that myfaces-shared had 
> tons of
>>>  duplicated code in it. One for core, one for tomahawk, one for 
> trinidad, etc.
>>>> 
>>>> 
>>>>   The shared part for core got moved to myfaces-core, but the deeper 
> problem
>>>  was that it was not easily possible to have multiple different versions 
> of
>>>  myfaces-shared. This now got solved by using the maven-shade-plugin. So 
> we
>>>  should rethink the practice to duplicate all the code and aim for a 
> _clean_
>>>  solution.
>>>> 
>>> 
>>>  Really that was not solved using maven-shade-plugin. What we did was
>>>  copy the code into myfaces-core and create a mirror of the same code
>>>  under shared. There, there is a profile called
>>>  "synch-myfaces-impl-shared", when it is added, the code is 
> copied and
>>>  then a manual commit do the trick.
>>> 
>>>>   Also (being a maven guy) I cannot quite follow the argument about 
> the
>>>  release cycles. Running a myfaces-shared release and then (with the 
> same staging
>>>  repo) a myfaces-core release is a task of 15 minutes. + the time for 
> running the
>>>  TCK, but this gets run via CI anyway, right? Thus this is barely a 
> problem.
>>>>   If it is then I'd happily volunteer to do the next release (do 
> this for
>>>  a few projects already) As you know, performing a release really got 
> _much_
>>>  easier nowadays with our new apache-parent pom.
>>>>   But maybe this argument was only meant for our old release process 
> (which I
>>>  agree was a lot of work)?
>>>> 
>>>>   If your answer is 'it's still needed' then can we just 
> unify
>>>  all other usages?
>>>> 
>>> 
>>>  Make a release is just the first of the problems. Take into account
>>>  that each release requires a vote and that vote takes 3 days to get
>>>  fixed. So in practice a problem in core can effectively block a
>>>  release of other artifacts. That's very inconvenient. Suppose we 
> have
>>>  a new TCK and that one found a problem on myfaces core. Again even if
>>>  the other artifacts are good enough, this becomes a blocker. There are
>>>  enough historical evidence that supports this point. In conclusion
>>>  this slow down the whole release cycle we have on myfaces. So ignore
>>>  that is not an option.
>>> 
>>>  Instead, maybe the option is reorganize myfaces core to allow
>>>  alternate release lifecycles per module. For example, each maven
>>>  plugin in myfaces has its own release lifecycle and there is a parent
>>>  pom with a different release procedure. This requires some changes to
>>>  create the source-release.zip file inherited from apache pom. But it
>>>  could be a cleaner solution.
>>> 
>>>  This means myfaces-commons project should be "merged" in some 
> way with
>>>  myfaces core. It has sense.
>>> 
>>>>   One question which bothers me with the 'shared' approach 
> if what
>>>  would happen to our build-tools annotation scanning 
> (@JSFWebConfigParam, etc)?
>>>  Does this already work with dependencies? Do we have this problem 
> already due to
>>>  the fact that we import such annotated classes via dependency?
>>>> 
>>> 
>>>  Those annotations comes from myfaces-builder-annotations. They are
>>>  source code annotations but all that information are saved on
>>>  myfaces-metadata.xml, so even if dissapear on compile time, the
>>>  information can be gathered from there. It is not a problem.
>>> 
>>>>>   Additionally, we increase the risk of "side 
> effects",
>>>>>   because a change done in core could introduce a bug in other 
> parts.
>>>>   Imo it's exactly the opposite. If you use the same code in 7 
> projects,
>>>  then it is more likely that a bug gets found and fixed.
>>>>   And the opposite case is (sadly) absolutely unlikely. If you have 
> a class
>>>  duplicated 7 times and find a bug in one project, it is highly unlikely 
> that all
>>>  6 other projects will get this fix applied :(
>>>> 
>>> 
>>>  But what happen when you have some code that does not have a clear
>>>  "interface". If somebody removes or change some code because 
> he/she
>>>  thinks it is not used in core or whatever, all 6 projects that could
>>>  require it will be affected and will require to rework its code.
>>>  Things get uglier when you have one library working with version 1.1.1
>>>  and 1.1.2 is binary incompatible with version 1.1.1, but my other
>>>  dependency requires it and kaboooom, the application does not work.
>>>  So, the first assumption we need to preserve in those 
> "shared"
>>>  artifacts is build it as an API, preserving binary compatibility.
>>> 
>>>  So we can't just grab the code from shared as is and say to users 
> "...
>>>  you can use that into its own projects ...". If the project is
>>>  maintained inside myfaces we can fix such problems, but outside
>>>  myfaces we should be more strict. So, we need a "public 
> shared" code
>>>  like the one proposed in myfaces commons and other code "myfaces
>>>  shared" to use in projects like tomahawk or portletbridge or 
> whatever
>>>  inside our land.
>>> 
>>>  regards,
>>> 
>>>  Leonardo Uribe
>>> 
>>>>   LieGrue,
>>>>   strub
>>>> 
>>>> 
>>>> 
>>>>   ----- Original Message -----
>>>>>   From: Leonardo Uribe <lu...@gmail.com>
>>>>>   To: MyFaces Development <de...@myfaces.apache.org>
>>>>>   Cc: Mark Struberg <st...@yahoo.de>
>>>>>   Sent: Sunday, October 23, 2011 9:08 PM
>>>>>   Subject: Re: [DISCUSS] how to get rid of tons of duplicated 
> code
>>>>> 
>>>>>   Hi
>>>>> 
>>>>>   Ok, let's check the proposal
>>>>> 
>>>>>   MS>> So the suggestion is:
>>>>>   MS>>
>>>>>   MS>> 1.) cleanup myfaces-shared. mf-shared has almost no
>>>  checkstyle
>>>>>   rules applied.
>>>>> 
>>>>>   Yes, sounds good.
>>>>> 
>>>>>   MS>> 2.) add unit tests for myfaces-shared. Currently 
> there are
>>>  not
>>>>>   many...
>>>>> 
>>>>>   Ok, sounds good too.
>>>>> 
>>>>>   MS>> 3.) move the shared code parts back to 
> myfaces-shared and
>>>  add unit
>>>>>   tests.
>>>>> 
>>>>>   So, this means do one step back and move the code from 
> myfaces-core
>>>>>   "shared" to myfaces-shared project? This breaks 
> effectively
>>>  the
>>>>>   changes done some months ago to make easier work with myfaces 
> core
>>>>>   itself.
>>>>> 
>>>>>   In that time the conclusion was: "core has priority over 
> anything
>>>>>   else, so shared code must live in core, but myfaces-shared 
> project
>>>>>   should just copy the code from there and have its own 
> lifecycle"
>>>>>   (these are my own words as I understood).
>>>>> 
>>>>>   So this point does not have practical sense, and go against 
> everything
>>>>>   discussed earlier.
>>>>> 
>>>>>   MS>> 4.) import myfaces-shared via maven dependency and 
> use
>>>>>   <minimizeJar> and <relocations> to package the 
> stuff
>>>>> 
>>>>>   maven-shade-plugin is a good "tool" but doesn't 
> fit well
>>>  in this
>>>>>   scenario. The reason is we need an alternate release lifecycle 
> for the
>>>>>   shared code between myfaces core and other projects.
>>>>> 
>>>>>   Historically that was the very first intention behind 
> myfaces-shared
>>>>>   project. Any myfaces core release requires some additional 
> steps to do
>>>>>   (TCK), so that becomes a problem when you try to release other
>>>>>   libraries that depends of shared. So, to fix that, 
> "shared"
>>>  was
>>>>>   created, so the code can be released in a independent way, and 
> prevent
>>>>>   myfaces core becomes an obstacle to release any other project
>>>>>   (tomahawk, portlet-bridge, ... ). So, to release tomahawk you 
> release
>>>>>   shared first and then tomahawk.
>>>>> 
>>>>>   maven-shade-plugin requires a released artifact to do its job. 
> So, use
>>>>>   it impose that restriction. In "shared" case, 
> preserve the
>>>  original
>>>>>   intention becomes "imperative", and that's the 
> reason why
>>>  a goal
>>>>>   was
>>>>>   created to copy the code from myfaces-core shared, so the 
> release
>>>>>   manager can run this goal, commit the changes and then run a 
> release.
>>>>> 
>>>>>   My proposal in this case is do the same we did for shared, but 
> for
>>>>>   "myfaces commons" case. Then we can use 
> maven-shade-plugin in
>>>  other
>>>>>   projects, but not over shared, instead over a released version 
> of
>>>>>   myfaces-commons-utils. Keep tomahawk or portlet-bridge as is, 
> using
>>>>>   shared project, because by its nature, those projects require 
> classes
>>>>>   that are not meant to be used outside those cases.
>>>>> 
>>>>>   Note do any hack in this part makes a little bit 
> "obscure"
>>>  how to make
>>>>>   changes, because everything becomes "centralized", 
> but makes
>>>  easier
>>>>>   maintain code. Additionally, we increase the risk of 
> "side
>>>  effects",
>>>>>   because a change done in core could introduce a bug in other 
> parts. So
>>>>>   at the end this is a matter of how to keep our code
>>>  "balanced", even
>>>>>   if some times it becomes a decision about "choose the 
> less
>>>>>   inconvenient alternative".
>>>>> 
>>>>>   regards,
>>>>> 
>>>>>   Leonardo Uribe
>>>>> 
>>>>>   2011/10/23 Leonardo Uribe <lu...@gmail.com>:
>>>>>>    Hi
>>>>>> 
>>>>>>    2011/10/23 Jakob Korherr <ja...@gmail.com>:
>>>>>>>    Hi Mark,
>>>>>>> 
>>>>>>>    +1 - that's exactly what I have been trying to 
> accomplish
>>>  some time
>>>>>>>    ago (introducing common-shades [1]). Unfortunately, I 
> was not
>>>>>>>    successful back then.
>>>>>>> 
>>>>>> 
>>>>>>    It is clear we need to "split" myfaces-impl 
> into
>>>  multiple
>>>>>   modules. There
>>>>>>    are some parts that are useful for other projects. The 
> code you
>>>  did
>>>>>>    on commons-shade was the attempt to solve the problem of 
> the
>>>>>>    duplicate code used on myfaces-test.
>>>>>> 
>>>>>>    Now the objective is find a way about how to reuse code 
> in myfaces
>>>>>>    core between multiple projects effectively.
>>>>>> 
>>>>>>>    However, there is a slight problem with moving all 
> this stuff
>>>  into
>>>>>>>    MyFaces shared, which I want to point out: code size. 
> If we
>>>  really put
>>>>>>>    all the code that is shared across any MyFaces 
> subproject into
>>>  shared,
>>>>>>>    it will get fat and ugly (even more than it is right 
> now). In
>>>>>>>    addition, if we continue including the whole shared 
> project
>>>  into
>>>>>>>    MyFaces core, MyFaces core impl will get bigger and 
> bigger.
>>>>>>> 
>>>>>> 
>>>>>>    Yes, the problem basically is MyFaces shared does not 
> have any
>>>  order
>>>>>>    or any notion of API. There are code that is used only in 
> tomahawk
>>>  but not
>>>>>>    intended to use in any other place. There are some useful
>>>  utitlities but
>>>>>>    sometimes without documentation, and there are some other 
> code
>>>  that is
>>>>>>    just obsolete. It it clear a cleanup of that location is
>>>>>>    necessary, but note priorities comes first, so this task 
> has been
>>>  delayed
>>>>>   in
>>>>>>    order to deal with other important stuff. Now it is a 
> good time to
>>>  fix
>>>>>   this.
>>>>>> 
>>>>>>>    Thus I'd like to suggest something similar which 
> I wanted
>>>  to
>>>>>>>    accomplish with common-shades: Introduce a new shared 
> module,
>>>  which
>>>>>>>    consists of many submodules that each handle a 
> specific
>>>  functionality
>>>>>>>    instead of being one fat module. With this approach 
> each
>>>  MyFaces
>>>>>>>    subproject would be able to pick out only the stuff 
> it really
>>>  needs.
>>>>>>>    Furthermore we would see more easily which code in 
> shared is
>>>  not used
>>>>>>>    anymore (I guess at the moment there is a lot of it), 
> just by
>>>  checking
>>>>>>>    which modules are still used in our poms.
>>>>>>> 
>>>>>> 
>>>>>>    That is the big question, how to split myfaces-impl and 
> shared.
>>>  Precisely
>>>>>>    the intention of myfaces-commons-utils projects was take 
> the stuff
>>>  that is
>>>>>>    useful from shared and build an usable API for developers 
> outside
>>>  MyFaces.
>>>>>> 
>>>>>>    For example, MyFaces HTML5 subproject was a good 
> experiment to see
>>>>>>    which code is useful and should be added in a API. Some 
> weeks ago
>>>  I checked
>>>>>>    and removed all duplicate code to use 
> myfaces-commons-utils. So
>>>  the 1.0.2
>>>>>>    release contains those classes taken from shared.
>>>>>> 
>>>>>>    regards,
>>>>>> 
>>>>>>    Leonardo Uribe
>>>>>> 
>>>>>>>    Regards,
>>>>>>>    Jakob
>>>>>>> 
>>>>>>>    [1] 
> https://svn.apache.org/repos/asf/myfaces/common-shades/
>>>>>>> 
>>>>>>>    2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>>>>>>    Hi!
>>>>>>>>    While working on the mafyces-commons cleanup I 
> figured
>>>  that we have
>>>>>   tons of
>>>>>>>>    duplicated code spread over MyFaces.
>>>>>>>> 
>>>>>>>> 
>>>>>>>>    As an example I like to mention
>>>  myfaces-commons-resourcehandler.
>>>>>   There are
>>>>>>>>    43 classes in total, and 35 of them are just 1:1 
> copied
>>>  from other
>>>>>   projects
>>>>>>>>    to provide resource management, zip, etc. For me 
> this is
>>>  an
>>>>>   absolute no-go.
>>>>>>>>    Those classes have neither tests nor any 
> documentation
>>>  where they
>>>>>   got forked
>>>>>>>>    from. Nor will any bug which gets fixed in 
> another module
>>>  make
>>>>>   it's way over
>>>>>>>>    to all the other projects containing that very 
> forked
>>>  code.
>>>>>   That's just ...
>>>>>>>>    unbelievable unmaintainable.
>>>>>>>> 
>>>>>>>>    There are 2 different ways to solve this 
> (depending on the
>>>>>   problem):
>>>>>>>> 
>>>>>>>>    A.) drop the functionality and provide a 
> generalized
>>>  solution. The
>>>>>   GZIP of
>>>>>>>>    myfaces-commons-resourcehandleris an obvious 
> example:
>>>>>>>>    We now copy this code over the 4th time or even 
> more.
>>>  Instead of
>>>>>   doing this,
>>>>>>>>    we should rather do it in the classic unix 
> fashion: do one
>>>  thing,
>>>>>   but do it
>>>>>>>>    well.
>>>>>>>>    Which means I'd rather see all the GZIP stuff 
> factored
>>>  out into
>>>>>   an own
>>>>>>>>    mf-commons module as a Servlet Filter. This can 
> then get
>>>  applied to
>>>>>   what
>>>>>>>>    ever other mechanism you like. This could also 
> (commonly)
>>>  cover
>>>>>   cases like
>>>>>>>>    detecting http UserAgents which are not able to 
> handle
>>>  zipped
>>>>>   resources,
>>>>>>>>    etc. That way we could provide this logic ONCE 
> and have
>>>  complete
>>>>>   freedom
>>>>>>>>    over the configuration.
>>>>>>>> 
>>>>>>>>    B.) code reusable components once and use them in 
> other
>>>  projects
>>>>>   (ev via
>>>>>>>>    shading it in).
>>>>>>>>    ClassLoaderResourceLoader would be a perfect 
> candidate! I
>>>  grepped
>>>>>   through
>>>>>>>>    only the few pits which I have checked out 
> locally and
>>>  found this
>>>>>   class 7
>>>>>>>>    SEVEN times! I just can't believe that we 
> can't
>>>  move this
>>>>>   stuff to a shared
>>>>>>>>    modul...
>>>>>>>> 
>>>>>>>>    Same for FacesServletMapping. 6 times copied 
> around,
>>>>>>>>    WebConfigProviderFactory 5 times, ...
>>>>>>>>    There are whole packages with 10++ classes which 
> got
>>>  copied 1:1!
>>>>>>>> 
>>>>>>>>    I really could cry seeing this :(
>>>>>>>> 
>>>>>>>> 
>>>>>>>>    What can we do to solve this?
>>>>>>>> 
>>>>>>>>    Theoretically myfaces-shared should contain this 
> stuff.
>>>  This is
>>>>>   exactly what
>>>>>>>>    it is for!
>>>>>>>>    Historically there have been some hand forged 
> tweeks and
>>>  ugly
>>>>>   hacks, but
>>>>>>>>    nowadays we have the maven-shade-plugin to make 
> our live
>>>  easier.
>>>>>>>> 
>>>>>>>>    So the suggestion is:
>>>>>>>> 
>>>>>>>>    1.) cleanup myfaces-shared. mf-shared has almost 
> no
>>>  checkstyle
>>>>>   rules
>>>>>>>>    applied.
>>>>>>>>    2.) add unit tests for myfaces-shared. Currently 
> there are
>>>  not
>>>>>   many...
>>>>>>>>    3.) move the shared code parts back to 
> myfaces-shared and
>>>  add unit
>>>>>   tests.
>>>>>>>>    4.) import myfaces-shared via maven dependency 
> and use
>>>>>   <minimizeJar> and
>>>>>>>>    <relocations> to package the stuff
>>>>>>>> 
>>>>>>>>    [+1] fine go ahead (ideally: yes, what parts can 
> I help
>>>  with?)
>>>>>>>>    [0] dont care
>>>>>>>>    [-1] wont work because ...
>>>>>>>> 
>>>>>>>> 
>>>>>>>>    I've attached a file which contains all 
> Classes which
>>>  name
>>>>>   exists multiple
>>>>>>>>    times in MyFaces. The number is the cound how 
> often they
>>>  exist in
>>>>>   MyFaces. I
>>>>>>>>    excluded current20.
>>>>>>>>    Please note that classes with the same name do 
> not
>>>  necessarily have
>>>>>   the same
>>>>>>>>    content - but quite a lot actually do have! 
> (scroll to the
>>>  bottom
>>>>>   of the
>>>>>>>>    file ...)
>>>>>>>> 
>>>>>>>>    LieGrue,
>>>>>>>>    strub
>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>>    --
>>>>>>>    Jakob Korherr
>>>>>>> 
>>>>>>>    blog: http://www.jakobk.com
>>>>>>>    twitter: http://twitter.com/jakobkorherr
>>>>>>>    work: http://www.irian.at
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>

Re: [DISCUSS] how to get rid of tons of duplicated code

[Leonardo Uribe <lu...@gmail.com>]

Hi

Ok, it is evident we have different opinions here about how to deal
with this problem. So, rather than refute the arguments I think it is
more productive to show another proposal. In MyFaces Core 2.0.x we
have the following layout:

api/
assembly/
bundle/
impl/
implee6/
integration-tests/
pom.xml
shared/
src/

Let's add two modules

parent : contains the parent POM that all submodules should inherit.
shared-utils : utilities for JSF used in core, but built as an api.

Now, we move the duplicate code related to myfaces-commons-utils into
this module, and shared will have shared-utils as dependency. impl
module will use maven-shade plugin to take the code from shared-utils
and shared (actually this is done for only for shared).

When a release of myfaces-core is done, all modules are released, so
things go as usual. But have the parent as module has the advantage
that if we want to release shared-utils or shared internal modules, we
can do it only releasing parent (optional) and shared-utils.

Since we can create a release for these modules, we remove the hack
used on on shared project (hard copy). Just we modify the pom to use
maven-shade-plugin over myfaces-core shared module. Then we kill
shared-tomahawk, and we make tomahawk use maven-shade-plugin over
shaded artifact in shared project.

In commons we do the same as in shared. In
myfaces-commons-resourcehandler we use myfaces core shared module
using maven-shade-plugin.

The only disadvantage is the release process gets a little bit more
complicated, but since do a release becomes easier with nexus, it is
ok.

Do you agree with this solution?

regards,

Leonardo Uribe

2011/10/24 Mark Struberg <st...@yahoo.de>:
>>Really that was not solved using maven-shade-plugin.
> Then we used the maven-shade-plugin the wrong way. See the <relocations> option [1].
>
>> There, there is a profile called
>> "synch-myfaces-impl-shared", when it is added, the code is copied and
>> then a manual commit do the trick.
>
> I think this is an ugly hack and doesn't solve any problems because
>
> a.)
>> Take into account
>> that each release requires a vote and that vote takes 3 days to get
>> fixed.
> you could just do a mvn release of shared + core in 1 go to the same staging repo -> only 1 vote is needed!
> This argument is simply wrong.
>
> b.)
> You
>  currently copy the code over 1:1 (half manually) thus your argument
> with 'core and other projects need different sources' is just nil. There
>  is no difference if you do this by profile, by hand or automatically!
> So I really prefer to have this automatically. Which is exactly what a
> dependency does...
>
> c.) the TCK argument is moot because it has
> nothing to do with shared. Most of the issues in the TCK are not
> affecting shared. And if they do, then those fixes are needed BY ALL
> OTHER PROJECTS AS WELL. Thus another argument against hiding this code
> and duplicating it all over...
>
> c.)
>> Instead, maybe the option is reorganize myfaces core to allow
>> alternate release lifecycles per module
> Sorry I don't grok this argument. It sounds as it would make all things more complicated without solving any real problem.
>
> e.)
>> This means myfaces-commons project should be "merged" in some way with
>> myfaces core. It has sense.
> 2 options:
> 1..) kill myfaces-shared completely and use the shared from core instead.
> Downside: if you need some fix in the shared code for some other project, you would need to release mf-core
> 2.) kill the newly introduced (this got only created a few weeks back by you) core-shared and use mf-shared again.
> Downside: hmmm nothing if one understands how to release correctly.
>
> f.)
>  all your explanations only explain the duplication between
> myfaces-shared and myfaces-core-shared. I can live with the duplication
> for now, but we also have classes which got copied around up to 8 times!
>  There is no excuse for that imo.
>
> g.)
>> But what happen when you have some code that does not have a clear
>> "interface". If somebody removes or change some code because he/she
>> thinks it is not used in core or whatever, all 6 projects that could
>> require it will be affected and will require to rework its code.
>> Things get uglier when you have one library working with version 1.1.1
>> and 1.1.2 is binary incompatible with version 1.1.1, but my other
>> dependency requires it and kaboooom, the application does not work.
>> So, the first assumption we need to preserve in those "shared"
>> artifacts is build it as an API, preserving binary compatibility.
>
> I don't get that argument neither!
> Hey,
>  that's life! If it turns out that the code is not good enough and needs
>  a fix, then that's the way it is! All other projects should fix that
> too in that case. I rather have a reproducible compile error which
> easily could get fixed than having tons of duplicated code which is more
>  or less always logically broken and badly tested.
> Yes, we should be
> aware that the classes we put into myfaces-shared must meet some
> standards and need to be well tested. But actually that would benefit
> our project a lot.
>
>
> h.) I just realised that our process in copying shared-impl from core to mf-shared is even more broken than every process before.
> If you are working on a lets say mf-commons project and find a bug in any of those shared parts, then you would need to RELEASE MF-CORE FIRST? omg, this cannot be serious!
>
>
> LieGrue,
> strub
>
>
> [1] http://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#relocations
>
>
>
> ----- Original Message -----
>> From: Leonardo Uribe <lu...@gmail.com>
>> To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg <st...@yahoo.de>
>> Cc:
>> Sent: Monday, October 24, 2011 4:36 AM
>> Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>
>> Hi
>>
>> 2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>  I've now read through the old mail archives and understand what the
>> original problem was. But actually I don't think we solved it correctly
>> right now. Of course we solved to original problem, but opened a can of worms
>> causing other problems.
>>>
>>>  The problem as far as I remember has been that myfaces-shared had tons of
>> duplicated code in it. One for core, one for tomahawk, one for trinidad, etc.
>>>
>>>
>>>  The shared part for core got moved to myfaces-core, but the deeper problem
>> was that it was not easily possible to have multiple different versions of
>> myfaces-shared. This now got solved by using the maven-shade-plugin. So we
>> should rethink the practice to duplicate all the code and aim for a _clean_
>> solution.
>>>
>>
>> Really that was not solved using maven-shade-plugin. What we did was
>> copy the code into myfaces-core and create a mirror of the same code
>> under shared. There, there is a profile called
>> "synch-myfaces-impl-shared", when it is added, the code is copied and
>> then a manual commit do the trick.
>>
>>>  Also (being a maven guy) I cannot quite follow the argument about the
>> release cycles. Running a myfaces-shared release and then (with the same staging
>> repo) a myfaces-core release is a task of 15 minutes. + the time for running the
>> TCK, but this gets run via CI anyway, right? Thus this is barely a problem.
>>>  If it is then I'd happily volunteer to do the next release (do this for
>> a few projects already) As you know, performing a release really got _much_
>> easier nowadays with our new apache-parent pom.
>>>  But maybe this argument was only meant for our old release process (which I
>> agree was a lot of work)?
>>>
>>>  If your answer is 'it's still needed' then can we just unify
>> all other usages?
>>>
>>
>> Make a release is just the first of the problems. Take into account
>> that each release requires a vote and that vote takes 3 days to get
>> fixed. So in practice a problem in core can effectively block a
>> release of other artifacts. That's very inconvenient. Suppose we have
>> a new TCK and that one found a problem on myfaces core. Again even if
>> the other artifacts are good enough, this becomes a blocker. There are
>> enough historical evidence that supports this point. In conclusion
>> this slow down the whole release cycle we have on myfaces. So ignore
>> that is not an option.
>>
>> Instead, maybe the option is reorganize myfaces core to allow
>> alternate release lifecycles per module. For example, each maven
>> plugin in myfaces has its own release lifecycle and there is a parent
>> pom with a different release procedure. This requires some changes to
>> create the source-release.zip file inherited from apache pom. But it
>> could be a cleaner solution.
>>
>> This means myfaces-commons project should be "merged" in some way with
>> myfaces core. It has sense.
>>
>>>  One question which bothers me with the 'shared' approach if what
>> would happen to our build-tools annotation scanning (@JSFWebConfigParam, etc)?
>> Does this already work with dependencies? Do we have this problem already due to
>> the fact that we import such annotated classes via dependency?
>>>
>>
>> Those annotations comes from myfaces-builder-annotations. They are
>> source code annotations but all that information are saved on
>> myfaces-metadata.xml, so even if dissapear on compile time, the
>> information can be gathered from there. It is not a problem.
>>
>>>>  Additionally, we increase the risk of "side effects",
>>>>  because a change done in core could introduce a bug in other parts.
>>>  Imo it's exactly the opposite. If you use the same code in 7 projects,
>> then it is more likely that a bug gets found and fixed.
>>>  And the opposite case is (sadly) absolutely unlikely. If you have a class
>> duplicated 7 times and find a bug in one project, it is highly unlikely that all
>> 6 other projects will get this fix applied :(
>>>
>>
>> But what happen when you have some code that does not have a clear
>> "interface". If somebody removes or change some code because he/she
>> thinks it is not used in core or whatever, all 6 projects that could
>> require it will be affected and will require to rework its code.
>> Things get uglier when you have one library working with version 1.1.1
>> and 1.1.2 is binary incompatible with version 1.1.1, but my other
>> dependency requires it and kaboooom, the application does not work.
>> So, the first assumption we need to preserve in those "shared"
>> artifacts is build it as an API, preserving binary compatibility.
>>
>> So we can't just grab the code from shared as is and say to users "...
>> you can use that into its own projects ...". If the project is
>> maintained inside myfaces we can fix such problems, but outside
>> myfaces we should be more strict. So, we need a "public shared" code
>> like the one proposed in myfaces commons and other code "myfaces
>> shared" to use in projects like tomahawk or portletbridge or whatever
>> inside our land.
>>
>> regards,
>>
>> Leonardo Uribe
>>
>>>  LieGrue,
>>>  strub
>>>
>>>
>>>
>>>  ----- Original Message -----
>>>>  From: Leonardo Uribe <lu...@gmail.com>
>>>>  To: MyFaces Development <de...@myfaces.apache.org>
>>>>  Cc: Mark Struberg <st...@yahoo.de>
>>>>  Sent: Sunday, October 23, 2011 9:08 PM
>>>>  Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>>>
>>>>  Hi
>>>>
>>>>  Ok, let's check the proposal
>>>>
>>>>  MS>> So the suggestion is:
>>>>  MS>>
>>>>  MS>> 1.) cleanup myfaces-shared. mf-shared has almost no
>> checkstyle
>>>>  rules applied.
>>>>
>>>>  Yes, sounds good.
>>>>
>>>>  MS>> 2.) add unit tests for myfaces-shared. Currently there are
>> not
>>>>  many...
>>>>
>>>>  Ok, sounds good too.
>>>>
>>>>  MS>> 3.) move the shared code parts back to myfaces-shared and
>> add unit
>>>>  tests.
>>>>
>>>>  So, this means do one step back and move the code from myfaces-core
>>>>  "shared" to myfaces-shared project? This breaks effectively
>> the
>>>>  changes done some months ago to make easier work with myfaces core
>>>>  itself.
>>>>
>>>>  In that time the conclusion was: "core has priority over anything
>>>>  else, so shared code must live in core, but myfaces-shared project
>>>>  should just copy the code from there and have its own lifecycle"
>>>>  (these are my own words as I understood).
>>>>
>>>>  So this point does not have practical sense, and go against everything
>>>>  discussed earlier.
>>>>
>>>>  MS>> 4.) import myfaces-shared via maven dependency and use
>>>>  <minimizeJar> and <relocations> to package the stuff
>>>>
>>>>  maven-shade-plugin is a good "tool" but doesn't fit well
>> in this
>>>>  scenario. The reason is we need an alternate release lifecycle for the
>>>>  shared code between myfaces core and other projects.
>>>>
>>>>  Historically that was the very first intention behind myfaces-shared
>>>>  project. Any myfaces core release requires some additional steps to do
>>>>  (TCK), so that becomes a problem when you try to release other
>>>>  libraries that depends of shared. So, to fix that, "shared"
>> was
>>>>  created, so the code can be released in a independent way, and prevent
>>>>  myfaces core becomes an obstacle to release any other project
>>>>  (tomahawk, portlet-bridge, ... ). So, to release tomahawk you release
>>>>  shared first and then tomahawk.
>>>>
>>>>  maven-shade-plugin requires a released artifact to do its job. So, use
>>>>  it impose that restriction. In "shared" case, preserve the
>> original
>>>>  intention becomes "imperative", and that's the reason why
>> a goal
>>>>  was
>>>>  created to copy the code from myfaces-core shared, so the release
>>>>  manager can run this goal, commit the changes and then run a release.
>>>>
>>>>  My proposal in this case is do the same we did for shared, but for
>>>>  "myfaces commons" case. Then we can use maven-shade-plugin in
>> other
>>>>  projects, but not over shared, instead over a released version of
>>>>  myfaces-commons-utils. Keep tomahawk or portlet-bridge as is, using
>>>>  shared project, because by its nature, those projects require classes
>>>>  that are not meant to be used outside those cases.
>>>>
>>>>  Note do any hack in this part makes a little bit "obscure"
>> how to make
>>>>  changes, because everything becomes "centralized", but makes
>> easier
>>>>  maintain code. Additionally, we increase the risk of "side
>> effects",
>>>>  because a change done in core could introduce a bug in other parts. So
>>>>  at the end this is a matter of how to keep our code
>> "balanced", even
>>>>  if some times it becomes a decision about "choose the less
>>>>  inconvenient alternative".
>>>>
>>>>  regards,
>>>>
>>>>  Leonardo Uribe
>>>>
>>>>  2011/10/23 Leonardo Uribe <lu...@gmail.com>:
>>>>>   Hi
>>>>>
>>>>>   2011/10/23 Jakob Korherr <ja...@gmail.com>:
>>>>>>   Hi Mark,
>>>>>>
>>>>>>   +1 - that's exactly what I have been trying to accomplish
>> some time
>>>>>>   ago (introducing common-shades [1]). Unfortunately, I was not
>>>>>>   successful back then.
>>>>>>
>>>>>
>>>>>   It is clear we need to "split" myfaces-impl into
>> multiple
>>>>  modules. There
>>>>>   are some parts that are useful for other projects. The code you
>> did
>>>>>   on commons-shade was the attempt to solve the problem of the
>>>>>   duplicate code used on myfaces-test.
>>>>>
>>>>>   Now the objective is find a way about how to reuse code in myfaces
>>>>>   core between multiple projects effectively.
>>>>>
>>>>>>   However, there is a slight problem with moving all this stuff
>> into
>>>>>>   MyFaces shared, which I want to point out: code size. If we
>> really put
>>>>>>   all the code that is shared across any MyFaces subproject into
>> shared,
>>>>>>   it will get fat and ugly (even more than it is right now). In
>>>>>>   addition, if we continue including the whole shared project
>> into
>>>>>>   MyFaces core, MyFaces core impl will get bigger and bigger.
>>>>>>
>>>>>
>>>>>   Yes, the problem basically is MyFaces shared does not have any
>> order
>>>>>   or any notion of API. There are code that is used only in tomahawk
>> but not
>>>>>   intended to use in any other place. There are some useful
>> utitlities but
>>>>>   sometimes without documentation, and there are some other code
>> that is
>>>>>   just obsolete. It it clear a cleanup of that location is
>>>>>   necessary, but note priorities comes first, so this task has been
>> delayed
>>>>  in
>>>>>   order to deal with other important stuff. Now it is a good time to
>> fix
>>>>  this.
>>>>>
>>>>>>   Thus I'd like to suggest something similar which I wanted
>> to
>>>>>>   accomplish with common-shades: Introduce a new shared module,
>> which
>>>>>>   consists of many submodules that each handle a specific
>> functionality
>>>>>>   instead of being one fat module. With this approach each
>> MyFaces
>>>>>>   subproject would be able to pick out only the stuff it really
>> needs.
>>>>>>   Furthermore we would see more easily which code in shared is
>> not used
>>>>>>   anymore (I guess at the moment there is a lot of it), just by
>> checking
>>>>>>   which modules are still used in our poms.
>>>>>>
>>>>>
>>>>>   That is the big question, how to split myfaces-impl and shared.
>> Precisely
>>>>>   the intention of myfaces-commons-utils projects was take the stuff
>> that is
>>>>>   useful from shared and build an usable API for developers outside
>> MyFaces.
>>>>>
>>>>>   For example, MyFaces HTML5 subproject was a good experiment to see
>>>>>   which code is useful and should be added in a API. Some weeks ago
>> I checked
>>>>>   and removed all duplicate code to use myfaces-commons-utils. So
>> the 1.0.2
>>>>>   release contains those classes taken from shared.
>>>>>
>>>>>   regards,
>>>>>
>>>>>   Leonardo Uribe
>>>>>
>>>>>>   Regards,
>>>>>>   Jakob
>>>>>>
>>>>>>   [1] https://svn.apache.org/repos/asf/myfaces/common-shades/
>>>>>>
>>>>>>   2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>>>>>   Hi!
>>>>>>>   While working on the mafyces-commons cleanup I figured
>> that we have
>>>>  tons of
>>>>>>>   duplicated code spread over MyFaces.
>>>>>>>
>>>>>>>
>>>>>>>   As an example I like to mention
>> myfaces-commons-resourcehandler.
>>>>  There are
>>>>>>>   43 classes in total, and 35 of them are just 1:1 copied
>> from other
>>>>  projects
>>>>>>>   to provide resource management, zip, etc. For me this is
>> an
>>>>  absolute no-go.
>>>>>>>   Those classes have neither tests nor any documentation
>> where they
>>>>  got forked
>>>>>>>   from. Nor will any bug which gets fixed in another module
>> make
>>>>  it's way over
>>>>>>>   to all the other projects containing that very forked
>> code.
>>>>  That's just ...
>>>>>>>   unbelievable unmaintainable.
>>>>>>>
>>>>>>>   There are 2 different ways to solve this (depending on the
>>>>  problem):
>>>>>>>
>>>>>>>   A.) drop the functionality and provide a generalized
>> solution. The
>>>>  GZIP of
>>>>>>>   myfaces-commons-resourcehandleris an obvious example:
>>>>>>>   We now copy this code over the 4th time or even more.
>> Instead of
>>>>  doing this,
>>>>>>>   we should rather do it in the classic unix fashion: do one
>> thing,
>>>>  but do it
>>>>>>>   well.
>>>>>>>   Which means I'd rather see all the GZIP stuff factored
>> out into
>>>>  an own
>>>>>>>   mf-commons module as a Servlet Filter. This can then get
>> applied to
>>>>  what
>>>>>>>   ever other mechanism you like. This could also (commonly)
>> cover
>>>>  cases like
>>>>>>>   detecting http UserAgents which are not able to handle
>> zipped
>>>>  resources,
>>>>>>>   etc. That way we could provide this logic ONCE and have
>> complete
>>>>  freedom
>>>>>>>   over the configuration.
>>>>>>>
>>>>>>>   B.) code reusable components once and use them in other
>> projects
>>>>  (ev via
>>>>>>>   shading it in).
>>>>>>>   ClassLoaderResourceLoader would be a perfect candidate! I
>> grepped
>>>>  through
>>>>>>>   only the few pits which I have checked out locally and
>> found this
>>>>  class 7
>>>>>>>   SEVEN times! I just can't believe that we can't
>> move this
>>>>  stuff to a shared
>>>>>>>   modul...
>>>>>>>
>>>>>>>   Same for FacesServletMapping. 6 times copied around,
>>>>>>>   WebConfigProviderFactory 5 times, ...
>>>>>>>   There are whole packages with 10++ classes which got
>> copied 1:1!
>>>>>>>
>>>>>>>   I really could cry seeing this :(
>>>>>>>
>>>>>>>
>>>>>>>   What can we do to solve this?
>>>>>>>
>>>>>>>   Theoretically myfaces-shared should contain this stuff.
>> This is
>>>>  exactly what
>>>>>>>   it is for!
>>>>>>>   Historically there have been some hand forged tweeks and
>> ugly
>>>>  hacks, but
>>>>>>>   nowadays we have the maven-shade-plugin to make our live
>> easier.
>>>>>>>
>>>>>>>   So the suggestion is:
>>>>>>>
>>>>>>>   1.) cleanup myfaces-shared. mf-shared has almost no
>> checkstyle
>>>>  rules
>>>>>>>   applied.
>>>>>>>   2.) add unit tests for myfaces-shared. Currently there are
>> not
>>>>  many...
>>>>>>>   3.) move the shared code parts back to myfaces-shared and
>> add unit
>>>>  tests.
>>>>>>>   4.) import myfaces-shared via maven dependency and use
>>>>  <minimizeJar> and
>>>>>>>   <relocations> to package the stuff
>>>>>>>
>>>>>>>   [+1] fine go ahead (ideally: yes, what parts can I help
>> with?)
>>>>>>>   [0] dont care
>>>>>>>   [-1] wont work because ...
>>>>>>>
>>>>>>>
>>>>>>>   I've attached a file which contains all Classes which
>> name
>>>>  exists multiple
>>>>>>>   times in MyFaces. The number is the cound how often they
>> exist in
>>>>  MyFaces. I
>>>>>>>   excluded current20.
>>>>>>>   Please note that classes with the same name do not
>> necessarily have
>>>>  the same
>>>>>>>   content - but quite a lot actually do have! (scroll to the
>> bottom
>>>>  of the
>>>>>>>   file ...)
>>>>>>>
>>>>>>>   LieGrue,
>>>>>>>   strub
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>   --
>>>>>>   Jakob Korherr
>>>>>>
>>>>>>   blog: http://www.jakobk.com
>>>>>>   twitter: http://twitter.com/jakobkorherr
>>>>>>   work: http://www.irian.at
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: [DISCUSS] how to get rid of tons of duplicated code

[Mark Struberg <st...@yahoo.de>]

>Really that was not solved using maven-shade-plugin. 
Then we used the maven-shade-plugin the wrong way. See the <relocations> option [1].

> There, there is a profile called
> "synch-myfaces-impl-shared", when it is added, the code is copied and
> then a manual commit do the trick.

I think this is an ugly hack and doesn't solve any problems because

a.) 
> Take into account
> that each release requires a vote and that vote takes 3 days to get
> fixed.
you could just do a mvn release of shared + core in 1 go to the same staging repo -> only 1 vote is needed!
This argument is simply wrong.

b.) 
You
 currently copy the code over 1:1 (half manually) thus your argument 
with 'core and other projects need different sources' is just nil. There
 is no difference if you do this by profile, by hand or automatically! 
So I really prefer to have this automatically. Which is exactly what a 
dependency does...

c.) the TCK argument is moot because it has 
nothing to do with shared. Most of the issues in the TCK are not 
affecting shared. And if they do, then those fixes are needed BY ALL 
OTHER PROJECTS AS WELL. Thus another argument against hiding this code 
and duplicating it all over...

c.)
> Instead, maybe the option is reorganize myfaces core to allow
> alternate release lifecycles per module
Sorry I don't grok this argument. It sounds as it would make all things more complicated without solving any real problem.

e.)
> This means myfaces-commons project should be "merged" in some way with
> myfaces core. It has sense.
2 options:
1..) kill myfaces-shared completely and use the shared from core instead.
Downside: if you need some fix in the shared code for some other project, you would need to release mf-core
2.) kill the newly introduced (this got only created a few weeks back by you) core-shared and use mf-shared again.
Downside: hmmm nothing if one understands how to release correctly.

f.)
 all your explanations only explain the duplication between 
myfaces-shared and myfaces-core-shared. I can live with the duplication 
for now, but we also have classes which got copied around up to 8 times!
 There is no excuse for that imo. 

g.)
> But what happen when you have some code that does not have a clear
> "interface". If somebody removes or change some code because he/she
> thinks it is not used in core or whatever, all 6 projects that could
> require it will be affected and will require to rework its code.
> Things get uglier when you have one library working with version 1.1.1
> and 1.1.2 is binary incompatible with version 1.1.1, but my other
> dependency requires it and kaboooom, the application does not work.
> So, the first assumption we need to preserve in those "shared"
> artifacts is build it as an API, preserving binary compatibility.

I don't get that argument neither!
Hey,
 that's life! If it turns out that the code is not good enough and needs
 a fix, then that's the way it is! All other projects should fix that 
too in that case. I rather have a reproducible compile error which 
easily could get fixed than having tons of duplicated code which is more
 or less always logically broken and badly tested.
Yes, we should be 
aware that the classes we put into myfaces-shared must meet some 
standards and need to be well tested. But actually that would benefit 
our project a lot.


h.) I just realised that our process in copying shared-impl from core to mf-shared is even more broken than every process before.
If you are working on a lets say mf-commons project and find a bug in any of those shared parts, then you would need to RELEASE MF-CORE FIRST? omg, this cannot be serious!


LieGrue,
strub


[1] http://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#relocations



----- Original Message -----
> From: Leonardo Uribe <lu...@gmail.com>
> To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg <st...@yahoo.de>
> Cc: 
> Sent: Monday, October 24, 2011 4:36 AM
> Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
> 
> Hi
> 
> 2011/10/23 Mark Struberg <st...@yahoo.de>:
>>  I've now read through the old mail archives and understand what the 
> original problem was. But actually I don't think we solved it correctly 
> right now. Of course we solved to original problem, but opened a can of worms 
> causing other problems.
>> 
>>  The problem as far as I remember has been that myfaces-shared had tons of 
> duplicated code in it. One for core, one for tomahawk, one for trinidad, etc.
>> 
>> 
>>  The shared part for core got moved to myfaces-core, but the deeper problem 
> was that it was not easily possible to have multiple different versions of 
> myfaces-shared. This now got solved by using the maven-shade-plugin. So we 
> should rethink the practice to duplicate all the code and aim for a _clean_ 
> solution.
>> 
> 
> Really that was not solved using maven-shade-plugin. What we did was
> copy the code into myfaces-core and create a mirror of the same code
> under shared. There, there is a profile called
> "synch-myfaces-impl-shared", when it is added, the code is copied and
> then a manual commit do the trick.
> 
>>  Also (being a maven guy) I cannot quite follow the argument about the 
> release cycles. Running a myfaces-shared release and then (with the same staging 
> repo) a myfaces-core release is a task of 15 minutes. + the time for running the 
> TCK, but this gets run via CI anyway, right? Thus this is barely a problem.
>>  If it is then I'd happily volunteer to do the next release (do this for 
> a few projects already) As you know, performing a release really got _much_ 
> easier nowadays with our new apache-parent pom.
>>  But maybe this argument was only meant for our old release process (which I 
> agree was a lot of work)?
>> 
>>  If your answer is 'it's still needed' then can we just unify 
> all other usages?
>> 
> 
> Make a release is just the first of the problems. Take into account
> that each release requires a vote and that vote takes 3 days to get
> fixed. So in practice a problem in core can effectively block a
> release of other artifacts. That's very inconvenient. Suppose we have
> a new TCK and that one found a problem on myfaces core. Again even if
> the other artifacts are good enough, this becomes a blocker. There are
> enough historical evidence that supports this point. In conclusion
> this slow down the whole release cycle we have on myfaces. So ignore
> that is not an option.
> 
> Instead, maybe the option is reorganize myfaces core to allow
> alternate release lifecycles per module. For example, each maven
> plugin in myfaces has its own release lifecycle and there is a parent
> pom with a different release procedure. This requires some changes to
> create the source-release.zip file inherited from apache pom. But it
> could be a cleaner solution.
> 
> This means myfaces-commons project should be "merged" in some way with
> myfaces core. It has sense.
> 
>>  One question which bothers me with the 'shared' approach if what 
> would happen to our build-tools annotation scanning (@JSFWebConfigParam, etc)? 
> Does this already work with dependencies? Do we have this problem already due to 
> the fact that we import such annotated classes via dependency?
>> 
> 
> Those annotations comes from myfaces-builder-annotations. They are
> source code annotations but all that information are saved on
> myfaces-metadata.xml, so even if dissapear on compile time, the
> information can be gathered from there. It is not a problem.
> 
>>>  Additionally, we increase the risk of "side effects",
>>>  because a change done in core could introduce a bug in other parts.
>>  Imo it's exactly the opposite. If you use the same code in 7 projects, 
> then it is more likely that a bug gets found and fixed.
>>  And the opposite case is (sadly) absolutely unlikely. If you have a class 
> duplicated 7 times and find a bug in one project, it is highly unlikely that all 
> 6 other projects will get this fix applied :(
>> 
> 
> But what happen when you have some code that does not have a clear
> "interface". If somebody removes or change some code because he/she
> thinks it is not used in core or whatever, all 6 projects that could
> require it will be affected and will require to rework its code.
> Things get uglier when you have one library working with version 1.1.1
> and 1.1.2 is binary incompatible with version 1.1.1, but my other
> dependency requires it and kaboooom, the application does not work.
> So, the first assumption we need to preserve in those "shared"
> artifacts is build it as an API, preserving binary compatibility.
> 
> So we can't just grab the code from shared as is and say to users "...
> you can use that into its own projects ...". If the project is
> maintained inside myfaces we can fix such problems, but outside
> myfaces we should be more strict. So, we need a "public shared" code
> like the one proposed in myfaces commons and other code "myfaces
> shared" to use in projects like tomahawk or portletbridge or whatever
> inside our land.
> 
> regards,
> 
> Leonardo Uribe
> 
>>  LieGrue,
>>  strub
>> 
>> 
>> 
>>  ----- Original Message -----
>>>  From: Leonardo Uribe <lu...@gmail.com>
>>>  To: MyFaces Development <de...@myfaces.apache.org>
>>>  Cc: Mark Struberg <st...@yahoo.de>
>>>  Sent: Sunday, October 23, 2011 9:08 PM
>>>  Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>> 
>>>  Hi
>>> 
>>>  Ok, let's check the proposal
>>> 
>>>  MS>> So the suggestion is:
>>>  MS>>
>>>  MS>> 1.) cleanup myfaces-shared. mf-shared has almost no 
> checkstyle
>>>  rules applied.
>>> 
>>>  Yes, sounds good.
>>> 
>>>  MS>> 2.) add unit tests for myfaces-shared. Currently there are 
> not
>>>  many...
>>> 
>>>  Ok, sounds good too.
>>> 
>>>  MS>> 3.) move the shared code parts back to myfaces-shared and 
> add unit
>>>  tests.
>>> 
>>>  So, this means do one step back and move the code from myfaces-core
>>>  "shared" to myfaces-shared project? This breaks effectively 
> the
>>>  changes done some months ago to make easier work with myfaces core
>>>  itself.
>>> 
>>>  In that time the conclusion was: "core has priority over anything
>>>  else, so shared code must live in core, but myfaces-shared project
>>>  should just copy the code from there and have its own lifecycle"
>>>  (these are my own words as I understood).
>>> 
>>>  So this point does not have practical sense, and go against everything
>>>  discussed earlier.
>>> 
>>>  MS>> 4.) import myfaces-shared via maven dependency and use
>>>  <minimizeJar> and <relocations> to package the stuff
>>> 
>>>  maven-shade-plugin is a good "tool" but doesn't fit well 
> in this
>>>  scenario. The reason is we need an alternate release lifecycle for the
>>>  shared code between myfaces core and other projects.
>>> 
>>>  Historically that was the very first intention behind myfaces-shared
>>>  project. Any myfaces core release requires some additional steps to do
>>>  (TCK), so that becomes a problem when you try to release other
>>>  libraries that depends of shared. So, to fix that, "shared" 
> was
>>>  created, so the code can be released in a independent way, and prevent
>>>  myfaces core becomes an obstacle to release any other project
>>>  (tomahawk, portlet-bridge, ... ). So, to release tomahawk you release
>>>  shared first and then tomahawk.
>>> 
>>>  maven-shade-plugin requires a released artifact to do its job. So, use
>>>  it impose that restriction. In "shared" case, preserve the 
> original
>>>  intention becomes "imperative", and that's the reason why 
> a goal
>>>  was
>>>  created to copy the code from myfaces-core shared, so the release
>>>  manager can run this goal, commit the changes and then run a release.
>>> 
>>>  My proposal in this case is do the same we did for shared, but for
>>>  "myfaces commons" case. Then we can use maven-shade-plugin in 
> other
>>>  projects, but not over shared, instead over a released version of
>>>  myfaces-commons-utils. Keep tomahawk or portlet-bridge as is, using
>>>  shared project, because by its nature, those projects require classes
>>>  that are not meant to be used outside those cases.
>>> 
>>>  Note do any hack in this part makes a little bit "obscure" 
> how to make
>>>  changes, because everything becomes "centralized", but makes 
> easier
>>>  maintain code. Additionally, we increase the risk of "side 
> effects",
>>>  because a change done in core could introduce a bug in other parts. So
>>>  at the end this is a matter of how to keep our code 
> "balanced", even
>>>  if some times it becomes a decision about "choose the less
>>>  inconvenient alternative".
>>> 
>>>  regards,
>>> 
>>>  Leonardo Uribe
>>> 
>>>  2011/10/23 Leonardo Uribe <lu...@gmail.com>:
>>>>   Hi
>>>> 
>>>>   2011/10/23 Jakob Korherr <ja...@gmail.com>:
>>>>>   Hi Mark,
>>>>> 
>>>>>   +1 - that's exactly what I have been trying to accomplish 
> some time
>>>>>   ago (introducing common-shades [1]). Unfortunately, I was not
>>>>>   successful back then.
>>>>> 
>>>> 
>>>>   It is clear we need to "split" myfaces-impl into 
> multiple
>>>  modules. There
>>>>   are some parts that are useful for other projects. The code you 
> did
>>>>   on commons-shade was the attempt to solve the problem of the
>>>>   duplicate code used on myfaces-test.
>>>> 
>>>>   Now the objective is find a way about how to reuse code in myfaces
>>>>   core between multiple projects effectively.
>>>> 
>>>>>   However, there is a slight problem with moving all this stuff 
> into
>>>>>   MyFaces shared, which I want to point out: code size. If we 
> really put
>>>>>   all the code that is shared across any MyFaces subproject into 
> shared,
>>>>>   it will get fat and ugly (even more than it is right now). In
>>>>>   addition, if we continue including the whole shared project 
> into
>>>>>   MyFaces core, MyFaces core impl will get bigger and bigger.
>>>>> 
>>>> 
>>>>   Yes, the problem basically is MyFaces shared does not have any 
> order
>>>>   or any notion of API. There are code that is used only in tomahawk 
> but not
>>>>   intended to use in any other place. There are some useful 
> utitlities but
>>>>   sometimes without documentation, and there are some other code 
> that is
>>>>   just obsolete. It it clear a cleanup of that location is
>>>>   necessary, but note priorities comes first, so this task has been 
> delayed
>>>  in
>>>>   order to deal with other important stuff. Now it is a good time to 
> fix
>>>  this.
>>>> 
>>>>>   Thus I'd like to suggest something similar which I wanted 
> to
>>>>>   accomplish with common-shades: Introduce a new shared module, 
> which
>>>>>   consists of many submodules that each handle a specific 
> functionality
>>>>>   instead of being one fat module. With this approach each 
> MyFaces
>>>>>   subproject would be able to pick out only the stuff it really 
> needs.
>>>>>   Furthermore we would see more easily which code in shared is 
> not used
>>>>>   anymore (I guess at the moment there is a lot of it), just by 
> checking
>>>>>   which modules are still used in our poms.
>>>>> 
>>>> 
>>>>   That is the big question, how to split myfaces-impl and shared. 
> Precisely
>>>>   the intention of myfaces-commons-utils projects was take the stuff 
> that is
>>>>   useful from shared and build an usable API for developers outside 
> MyFaces.
>>>> 
>>>>   For example, MyFaces HTML5 subproject was a good experiment to see
>>>>   which code is useful and should be added in a API. Some weeks ago 
> I checked
>>>>   and removed all duplicate code to use myfaces-commons-utils. So 
> the 1.0.2
>>>>   release contains those classes taken from shared.
>>>> 
>>>>   regards,
>>>> 
>>>>   Leonardo Uribe
>>>> 
>>>>>   Regards,
>>>>>   Jakob
>>>>> 
>>>>>   [1] https://svn.apache.org/repos/asf/myfaces/common-shades/
>>>>> 
>>>>>   2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>>>>   Hi!
>>>>>>   While working on the mafyces-commons cleanup I figured 
> that we have
>>>  tons of
>>>>>>   duplicated code spread over MyFaces.
>>>>>> 
>>>>>> 
>>>>>>   As an example I like to mention 
> myfaces-commons-resourcehandler.
>>>  There are
>>>>>>   43 classes in total, and 35 of them are just 1:1 copied 
> from other
>>>  projects
>>>>>>   to provide resource management, zip, etc. For me this is 
> an
>>>  absolute no-go.
>>>>>>   Those classes have neither tests nor any documentation 
> where they
>>>  got forked
>>>>>>   from. Nor will any bug which gets fixed in another module 
> make
>>>  it's way over
>>>>>>   to all the other projects containing that very forked 
> code.
>>>  That's just ...
>>>>>>   unbelievable unmaintainable.
>>>>>> 
>>>>>>   There are 2 different ways to solve this (depending on the
>>>  problem):
>>>>>> 
>>>>>>   A.) drop the functionality and provide a generalized 
> solution. The
>>>  GZIP of
>>>>>>   myfaces-commons-resourcehandleris an obvious example:
>>>>>>   We now copy this code over the 4th time or even more. 
> Instead of
>>>  doing this,
>>>>>>   we should rather do it in the classic unix fashion: do one 
> thing,
>>>  but do it
>>>>>>   well.
>>>>>>   Which means I'd rather see all the GZIP stuff factored 
> out into
>>>  an own
>>>>>>   mf-commons module as a Servlet Filter. This can then get 
> applied to
>>>  what
>>>>>>   ever other mechanism you like. This could also (commonly) 
> cover
>>>  cases like
>>>>>>   detecting http UserAgents which are not able to handle 
> zipped
>>>  resources,
>>>>>>   etc. That way we could provide this logic ONCE and have 
> complete
>>>  freedom
>>>>>>   over the configuration.
>>>>>> 
>>>>>>   B.) code reusable components once and use them in other 
> projects
>>>  (ev via
>>>>>>   shading it in).
>>>>>>   ClassLoaderResourceLoader would be a perfect candidate! I 
> grepped
>>>  through
>>>>>>   only the few pits which I have checked out locally and 
> found this
>>>  class 7
>>>>>>   SEVEN times! I just can't believe that we can't 
> move this
>>>  stuff to a shared
>>>>>>   modul...
>>>>>> 
>>>>>>   Same for FacesServletMapping. 6 times copied around,
>>>>>>   WebConfigProviderFactory 5 times, ...
>>>>>>   There are whole packages with 10++ classes which got 
> copied 1:1!
>>>>>> 
>>>>>>   I really could cry seeing this :(
>>>>>> 
>>>>>> 
>>>>>>   What can we do to solve this?
>>>>>> 
>>>>>>   Theoretically myfaces-shared should contain this stuff. 
> This is
>>>  exactly what
>>>>>>   it is for!
>>>>>>   Historically there have been some hand forged tweeks and 
> ugly
>>>  hacks, but
>>>>>>   nowadays we have the maven-shade-plugin to make our live 
> easier.
>>>>>> 
>>>>>>   So the suggestion is:
>>>>>> 
>>>>>>   1.) cleanup myfaces-shared. mf-shared has almost no 
> checkstyle
>>>  rules
>>>>>>   applied.
>>>>>>   2.) add unit tests for myfaces-shared. Currently there are 
> not
>>>  many...
>>>>>>   3.) move the shared code parts back to myfaces-shared and 
> add unit
>>>  tests.
>>>>>>   4.) import myfaces-shared via maven dependency and use
>>>  <minimizeJar> and
>>>>>>   <relocations> to package the stuff
>>>>>> 
>>>>>>   [+1] fine go ahead (ideally: yes, what parts can I help 
> with?)
>>>>>>   [0] dont care
>>>>>>   [-1] wont work because ...
>>>>>> 
>>>>>> 
>>>>>>   I've attached a file which contains all Classes which 
> name
>>>  exists multiple
>>>>>>   times in MyFaces. The number is the cound how often they 
> exist in
>>>  MyFaces. I
>>>>>>   excluded current20.
>>>>>>   Please note that classes with the same name do not 
> necessarily have
>>>  the same
>>>>>>   content - but quite a lot actually do have! (scroll to the 
> bottom
>>>  of the
>>>>>>   file ...)
>>>>>> 
>>>>>>   LieGrue,
>>>>>>   strub
>>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>>   --
>>>>>   Jakob Korherr
>>>>> 
>>>>>   blog: http://www.jakobk.com
>>>>>   twitter: http://twitter.com/jakobkorherr
>>>>>   work: http://www.irian.at
>>>>> 
>>>> 
>>> 
>> 
>

Re: [DISCUSS] how to get rid of tons of duplicated code

[Leonardo Uribe <lu...@gmail.com>]

Hi

2011/10/23 Mark Struberg <st...@yahoo.de>:
> I've now read through the old mail archives and understand what the original problem was. But actually I don't think we solved it correctly right now. Of course we solved to original problem, but opened a can of worms causing other problems.
>
> The problem as far as I remember has been that myfaces-shared had tons of duplicated code in it. One for core, one for tomahawk, one for trinidad, etc.
>
>
> The shared part for core got moved to myfaces-core, but the deeper problem was that it was not easily possible to have multiple different versions of myfaces-shared. This now got solved by using the maven-shade-plugin. So we should rethink the practice to duplicate all the code and aim for a _clean_ solution.
>

Really that was not solved using maven-shade-plugin. What we did was
copy the code into myfaces-core and create a mirror of the same code
under shared. There, there is a profile called
"synch-myfaces-impl-shared", when it is added, the code is copied and
then a manual commit do the trick.

> Also (being a maven guy) I cannot quite follow the argument about the release cycles. Running a myfaces-shared release and then (with the same staging repo) a myfaces-core release is a task of 15 minutes. + the time for running the TCK, but this gets run via CI anyway, right? Thus this is barely a problem.
> If it is then I'd happily volunteer to do the next release (do this for a few projects already) As you know, performing a release really got _much_ easier nowadays with our new apache-parent pom.
> But maybe this argument was only meant for our old release process (which I agree was a lot of work)?
>
> If your answer is 'it's still needed' then can we just unify all other usages?
>

Make a release is just the first of the problems. Take into account
that each release requires a vote and that vote takes 3 days to get
fixed. So in practice a problem in core can effectively block a
release of other artifacts. That's very inconvenient. Suppose we have
a new TCK and that one found a problem on myfaces core. Again even if
the other artifacts are good enough, this becomes a blocker. There are
enough historical evidence that supports this point. In conclusion
this slow down the whole release cycle we have on myfaces. So ignore
that is not an option.

Instead, maybe the option is reorganize myfaces core to allow
alternate release lifecycles per module. For example, each maven
plugin in myfaces has its own release lifecycle and there is a parent
pom with a different release procedure. This requires some changes to
create the source-release.zip file inherited from apache pom. But it
could be a cleaner solution.

This means myfaces-commons project should be "merged" in some way with
myfaces core. It has sense.

> One question which bothers me with the 'shared' approach if what would happen to our build-tools annotation scanning (@JSFWebConfigParam, etc)? Does this already work with dependencies? Do we have this problem already due to the fact that we import such annotated classes via dependency?
>

Those annotations comes from myfaces-builder-annotations. They are
source code annotations but all that information are saved on
myfaces-metadata.xml, so even if dissapear on compile time, the
information can be gathered from there. It is not a problem.

>> Additionally, we increase the risk of "side effects",
>> because a change done in core could introduce a bug in other parts.
> Imo it's exactly the opposite. If you use the same code in 7 projects, then it is more likely that a bug gets found and fixed.
> And the opposite case is (sadly) absolutely unlikely. If you have a class duplicated 7 times and find a bug in one project, it is highly unlikely that all 6 other projects will get this fix applied :(
>

But what happen when you have some code that does not have a clear
"interface". If somebody removes or change some code because he/she
thinks it is not used in core or whatever, all 6 projects that could
require it will be affected and will require to rework its code.
Things get uglier when you have one library working with version 1.1.1
and 1.1.2 is binary incompatible with version 1.1.1, but my other
dependency requires it and kaboooom, the application does not work.
So, the first assumption we need to preserve in those "shared"
artifacts is build it as an API, preserving binary compatibility.

So we can't just grab the code from shared as is and say to users "...
you can use that into its own projects ...". If the project is
maintained inside myfaces we can fix such problems, but outside
myfaces we should be more strict. So, we need a "public shared" code
like the one proposed in myfaces commons and other code "myfaces
shared" to use in projects like tomahawk or portletbridge or whatever
inside our land.

regards,

Leonardo Uribe

> LieGrue,
> strub
>
>
>
> ----- Original Message -----
>> From: Leonardo Uribe <lu...@gmail.com>
>> To: MyFaces Development <de...@myfaces.apache.org>
>> Cc: Mark Struberg <st...@yahoo.de>
>> Sent: Sunday, October 23, 2011 9:08 PM
>> Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>
>> Hi
>>
>> Ok, let's check the proposal
>>
>> MS>> So the suggestion is:
>> MS>>
>> MS>> 1.) cleanup myfaces-shared. mf-shared has almost no checkstyle
>> rules applied.
>>
>> Yes, sounds good.
>>
>> MS>> 2.) add unit tests for myfaces-shared. Currently there are not
>> many...
>>
>> Ok, sounds good too.
>>
>> MS>> 3.) move the shared code parts back to myfaces-shared and add unit
>> tests.
>>
>> So, this means do one step back and move the code from myfaces-core
>> "shared" to myfaces-shared project? This breaks effectively the
>> changes done some months ago to make easier work with myfaces core
>> itself.
>>
>> In that time the conclusion was: "core has priority over anything
>> else, so shared code must live in core, but myfaces-shared project
>> should just copy the code from there and have its own lifecycle"
>> (these are my own words as I understood).
>>
>> So this point does not have practical sense, and go against everything
>> discussed earlier.
>>
>> MS>> 4.) import myfaces-shared via maven dependency and use
>> <minimizeJar> and <relocations> to package the stuff
>>
>> maven-shade-plugin is a good "tool" but doesn't fit well in this
>> scenario. The reason is we need an alternate release lifecycle for the
>> shared code between myfaces core and other projects.
>>
>> Historically that was the very first intention behind myfaces-shared
>> project. Any myfaces core release requires some additional steps to do
>> (TCK), so that becomes a problem when you try to release other
>> libraries that depends of shared. So, to fix that, "shared" was
>> created, so the code can be released in a independent way, and prevent
>> myfaces core becomes an obstacle to release any other project
>> (tomahawk, portlet-bridge, ... ). So, to release tomahawk you release
>> shared first and then tomahawk.
>>
>> maven-shade-plugin requires a released artifact to do its job. So, use
>> it impose that restriction. In "shared" case, preserve the original
>> intention becomes "imperative", and that's the reason why a goal
>> was
>> created to copy the code from myfaces-core shared, so the release
>> manager can run this goal, commit the changes and then run a release.
>>
>> My proposal in this case is do the same we did for shared, but for
>> "myfaces commons" case. Then we can use maven-shade-plugin in other
>> projects, but not over shared, instead over a released version of
>> myfaces-commons-utils. Keep tomahawk or portlet-bridge as is, using
>> shared project, because by its nature, those projects require classes
>> that are not meant to be used outside those cases.
>>
>> Note do any hack in this part makes a little bit "obscure" how to make
>> changes, because everything becomes "centralized", but makes easier
>> maintain code. Additionally, we increase the risk of "side effects",
>> because a change done in core could introduce a bug in other parts. So
>> at the end this is a matter of how to keep our code "balanced", even
>> if some times it becomes a decision about "choose the less
>> inconvenient alternative".
>>
>> regards,
>>
>> Leonardo Uribe
>>
>> 2011/10/23 Leonardo Uribe <lu...@gmail.com>:
>>>  Hi
>>>
>>>  2011/10/23 Jakob Korherr <ja...@gmail.com>:
>>>>  Hi Mark,
>>>>
>>>>  +1 - that's exactly what I have been trying to accomplish some time
>>>>  ago (introducing common-shades [1]). Unfortunately, I was not
>>>>  successful back then.
>>>>
>>>
>>>  It is clear we need to "split" myfaces-impl into multiple
>> modules. There
>>>  are some parts that are useful for other projects. The code you did
>>>  on commons-shade was the attempt to solve the problem of the
>>>  duplicate code used on myfaces-test.
>>>
>>>  Now the objective is find a way about how to reuse code in myfaces
>>>  core between multiple projects effectively.
>>>
>>>>  However, there is a slight problem with moving all this stuff into
>>>>  MyFaces shared, which I want to point out: code size. If we really put
>>>>  all the code that is shared across any MyFaces subproject into shared,
>>>>  it will get fat and ugly (even more than it is right now). In
>>>>  addition, if we continue including the whole shared project into
>>>>  MyFaces core, MyFaces core impl will get bigger and bigger.
>>>>
>>>
>>>  Yes, the problem basically is MyFaces shared does not have any order
>>>  or any notion of API. There are code that is used only in tomahawk but not
>>>  intended to use in any other place. There are some useful utitlities but
>>>  sometimes without documentation, and there are some other code that is
>>>  just obsolete. It it clear a cleanup of that location is
>>>  necessary, but note priorities comes first, so this task has been delayed
>> in
>>>  order to deal with other important stuff. Now it is a good time to fix
>> this.
>>>
>>>>  Thus I'd like to suggest something similar which I wanted to
>>>>  accomplish with common-shades: Introduce a new shared module, which
>>>>  consists of many submodules that each handle a specific functionality
>>>>  instead of being one fat module. With this approach each MyFaces
>>>>  subproject would be able to pick out only the stuff it really needs.
>>>>  Furthermore we would see more easily which code in shared is not used
>>>>  anymore (I guess at the moment there is a lot of it), just by checking
>>>>  which modules are still used in our poms.
>>>>
>>>
>>>  That is the big question, how to split myfaces-impl and shared. Precisely
>>>  the intention of myfaces-commons-utils projects was take the stuff that is
>>>  useful from shared and build an usable API for developers outside MyFaces.
>>>
>>>  For example, MyFaces HTML5 subproject was a good experiment to see
>>>  which code is useful and should be added in a API. Some weeks ago I checked
>>>  and removed all duplicate code to use myfaces-commons-utils. So the 1.0.2
>>>  release contains those classes taken from shared.
>>>
>>>  regards,
>>>
>>>  Leonardo Uribe
>>>
>>>>  Regards,
>>>>  Jakob
>>>>
>>>>  [1] https://svn.apache.org/repos/asf/myfaces/common-shades/
>>>>
>>>>  2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>>>  Hi!
>>>>>  While working on the mafyces-commons cleanup I figured that we have
>> tons of
>>>>>  duplicated code spread over MyFaces.
>>>>>
>>>>>
>>>>>  As an example I like to mention myfaces-commons-resourcehandler.
>> There are
>>>>>  43 classes in total, and 35 of them are just 1:1 copied from other
>> projects
>>>>>  to provide resource management, zip, etc. For me this is an
>> absolute no-go.
>>>>>  Those classes have neither tests nor any documentation where they
>> got forked
>>>>>  from. Nor will any bug which gets fixed in another module make
>> it's way over
>>>>>  to all the other projects containing that very forked code.
>> That's just ...
>>>>>  unbelievable unmaintainable.
>>>>>
>>>>>  There are 2 different ways to solve this (depending on the
>> problem):
>>>>>
>>>>>  A.) drop the functionality and provide a generalized solution. The
>> GZIP of
>>>>>  myfaces-commons-resourcehandleris an obvious example:
>>>>>  We now copy this code over the 4th time or even more. Instead of
>> doing this,
>>>>>  we should rather do it in the classic unix fashion: do one thing,
>> but do it
>>>>>  well.
>>>>>  Which means I'd rather see all the GZIP stuff factored out into
>> an own
>>>>>  mf-commons module as a Servlet Filter. This can then get applied to
>> what
>>>>>  ever other mechanism you like. This could also (commonly) cover
>> cases like
>>>>>  detecting http UserAgents which are not able to handle zipped
>> resources,
>>>>>  etc. That way we could provide this logic ONCE and have complete
>> freedom
>>>>>  over the configuration.
>>>>>
>>>>>  B.) code reusable components once and use them in other projects
>> (ev via
>>>>>  shading it in).
>>>>>  ClassLoaderResourceLoader would be a perfect candidate! I grepped
>> through
>>>>>  only the few pits which I have checked out locally and found this
>> class 7
>>>>>  SEVEN times! I just can't believe that we can't move this
>> stuff to a shared
>>>>>  modul...
>>>>>
>>>>>  Same for FacesServletMapping. 6 times copied around,
>>>>>  WebConfigProviderFactory 5 times, ...
>>>>>  There are whole packages with 10++ classes which got copied 1:1!
>>>>>
>>>>>  I really could cry seeing this :(
>>>>>
>>>>>
>>>>>  What can we do to solve this?
>>>>>
>>>>>  Theoretically myfaces-shared should contain this stuff. This is
>> exactly what
>>>>>  it is for!
>>>>>  Historically there have been some hand forged tweeks and ugly
>> hacks, but
>>>>>  nowadays we have the maven-shade-plugin to make our live easier.
>>>>>
>>>>>  So the suggestion is:
>>>>>
>>>>>  1.) cleanup myfaces-shared. mf-shared has almost no checkstyle
>> rules
>>>>>  applied.
>>>>>  2.) add unit tests for myfaces-shared. Currently there are not
>> many...
>>>>>  3.) move the shared code parts back to myfaces-shared and add unit
>> tests.
>>>>>  4.) import myfaces-shared via maven dependency and use
>> <minimizeJar> and
>>>>>  <relocations> to package the stuff
>>>>>
>>>>>  [+1] fine go ahead (ideally: yes, what parts can I help with?)
>>>>>  [0] dont care
>>>>>  [-1] wont work because ...
>>>>>
>>>>>
>>>>>  I've attached a file which contains all Classes which name
>> exists multiple
>>>>>  times in MyFaces. The number is the cound how often they exist in
>> MyFaces. I
>>>>>  excluded current20.
>>>>>  Please note that classes with the same name do not necessarily have
>> the same
>>>>>  content - but quite a lot actually do have! (scroll to the bottom
>> of the
>>>>>  file ...)
>>>>>
>>>>>  LieGrue,
>>>>>  strub
>>>>>
>>>>
>>>>
>>>>
>>>>  --
>>>>  Jakob Korherr
>>>>
>>>>  blog: http://www.jakobk.com
>>>>  twitter: http://twitter.com/jakobkorherr
>>>>  work: http://www.irian.at
>>>>
>>>
>>
>

Re: [DISCUSS] how to get rid of tons of duplicated code

[Mark Struberg <st...@yahoo.de>]

I've now read through the old mail archives and understand what the original problem was. But actually I don't think we solved it correctly right now. Of course we solved to original problem, but opened a can of worms causing other problems.

The problem as far as I remember has been that myfaces-shared had tons of duplicated code in it. One for core, one for tomahawk, one for trinidad, etc. 


The shared part for core got moved to myfaces-core, but the deeper problem was that it was not easily possible to have multiple different versions of myfaces-shared. This now got solved by using the maven-shade-plugin. So we should rethink the practice to duplicate all the code and aim for a _clean_ solution.

Also (being a maven guy) I cannot quite follow the argument about the release cycles. Running a myfaces-shared release and then (with the same staging repo) a myfaces-core release is a task of 15 minutes. + the time for running the TCK, but this gets run via CI anyway, right? Thus this is barely a problem.
If it is then I'd happily volunteer to do the next release (do this for a few projects already) As you know, performing a release really got _much_ easier nowadays with our new apache-parent pom.
But maybe this argument was only meant for our old release process (which I agree was a lot of work)?

If your answer is 'it's still needed' then can we just unify all other usages?

One question which bothers me with the 'shared' approach if what would happen to our build-tools annotation scanning (@JSFWebConfigParam, etc)? Does this already work with dependencies? Do we have this problem already due to the fact that we import such annotated classes via dependency?

> Additionally, we increase the risk of "side effects",
> because a change done in core could introduce a bug in other parts.
Imo it's exactly the opposite. If you use the same code in 7 projects, then it is more likely that a bug gets found and fixed. 
And the opposite case is (sadly) absolutely unlikely. If you have a class duplicated 7 times and find a bug in one project, it is highly unlikely that all 6 other projects will get this fix applied :(

LieGrue,
strub



----- Original Message -----
> From: Leonardo Uribe <lu...@gmail.com>
> To: MyFaces Development <de...@myfaces.apache.org>
> Cc: Mark Struberg <st...@yahoo.de>
> Sent: Sunday, October 23, 2011 9:08 PM
> Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
> 
> Hi
> 
> Ok, let's check the proposal
> 
> MS>> So the suggestion is:
> MS>>
> MS>> 1.) cleanup myfaces-shared. mf-shared has almost no checkstyle
> rules applied.
> 
> Yes, sounds good.
> 
> MS>> 2.) add unit tests for myfaces-shared. Currently there are not 
> many...
> 
> Ok, sounds good too.
> 
> MS>> 3.) move the shared code parts back to myfaces-shared and add unit 
> tests.
> 
> So, this means do one step back and move the code from myfaces-core
> "shared" to myfaces-shared project? This breaks effectively the
> changes done some months ago to make easier work with myfaces core
> itself.
> 
> In that time the conclusion was: "core has priority over anything
> else, so shared code must live in core, but myfaces-shared project
> should just copy the code from there and have its own lifecycle"
> (these are my own words as I understood).
> 
> So this point does not have practical sense, and go against everything
> discussed earlier.
> 
> MS>> 4.) import myfaces-shared via maven dependency and use
> <minimizeJar> and <relocations> to package the stuff
> 
> maven-shade-plugin is a good "tool" but doesn't fit well in this
> scenario. The reason is we need an alternate release lifecycle for the
> shared code between myfaces core and other projects.
> 
> Historically that was the very first intention behind myfaces-shared
> project. Any myfaces core release requires some additional steps to do
> (TCK), so that becomes a problem when you try to release other
> libraries that depends of shared. So, to fix that, "shared" was
> created, so the code can be released in a independent way, and prevent
> myfaces core becomes an obstacle to release any other project
> (tomahawk, portlet-bridge, ... ). So, to release tomahawk you release
> shared first and then tomahawk.
> 
> maven-shade-plugin requires a released artifact to do its job. So, use
> it impose that restriction. In "shared" case, preserve the original
> intention becomes "imperative", and that's the reason why a goal 
> was
> created to copy the code from myfaces-core shared, so the release
> manager can run this goal, commit the changes and then run a release.
> 
> My proposal in this case is do the same we did for shared, but for
> "myfaces commons" case. Then we can use maven-shade-plugin in other
> projects, but not over shared, instead over a released version of
> myfaces-commons-utils. Keep tomahawk or portlet-bridge as is, using
> shared project, because by its nature, those projects require classes
> that are not meant to be used outside those cases.
> 
> Note do any hack in this part makes a little bit "obscure" how to make
> changes, because everything becomes "centralized", but makes easier
> maintain code. Additionally, we increase the risk of "side effects",
> because a change done in core could introduce a bug in other parts. So
> at the end this is a matter of how to keep our code "balanced", even
> if some times it becomes a decision about "choose the less
> inconvenient alternative".
> 
> regards,
> 
> Leonardo Uribe
> 
> 2011/10/23 Leonardo Uribe <lu...@gmail.com>:
>>  Hi
>> 
>>  2011/10/23 Jakob Korherr <ja...@gmail.com>:
>>>  Hi Mark,
>>> 
>>>  +1 - that's exactly what I have been trying to accomplish some time
>>>  ago (introducing common-shades [1]). Unfortunately, I was not
>>>  successful back then.
>>> 
>> 
>>  It is clear we need to "split" myfaces-impl into multiple 
> modules. There
>>  are some parts that are useful for other projects. The code you did
>>  on commons-shade was the attempt to solve the problem of the
>>  duplicate code used on myfaces-test.
>> 
>>  Now the objective is find a way about how to reuse code in myfaces
>>  core between multiple projects effectively.
>> 
>>>  However, there is a slight problem with moving all this stuff into
>>>  MyFaces shared, which I want to point out: code size. If we really put
>>>  all the code that is shared across any MyFaces subproject into shared,
>>>  it will get fat and ugly (even more than it is right now). In
>>>  addition, if we continue including the whole shared project into
>>>  MyFaces core, MyFaces core impl will get bigger and bigger.
>>> 
>> 
>>  Yes, the problem basically is MyFaces shared does not have any order
>>  or any notion of API. There are code that is used only in tomahawk but not
>>  intended to use in any other place. There are some useful utitlities but
>>  sometimes without documentation, and there are some other code that is
>>  just obsolete. It it clear a cleanup of that location is
>>  necessary, but note priorities comes first, so this task has been delayed 
> in
>>  order to deal with other important stuff. Now it is a good time to fix 
> this.
>> 
>>>  Thus I'd like to suggest something similar which I wanted to
>>>  accomplish with common-shades: Introduce a new shared module, which
>>>  consists of many submodules that each handle a specific functionality
>>>  instead of being one fat module. With this approach each MyFaces
>>>  subproject would be able to pick out only the stuff it really needs.
>>>  Furthermore we would see more easily which code in shared is not used
>>>  anymore (I guess at the moment there is a lot of it), just by checking
>>>  which modules are still used in our poms.
>>> 
>> 
>>  That is the big question, how to split myfaces-impl and shared. Precisely
>>  the intention of myfaces-commons-utils projects was take the stuff that is
>>  useful from shared and build an usable API for developers outside MyFaces.
>> 
>>  For example, MyFaces HTML5 subproject was a good experiment to see
>>  which code is useful and should be added in a API. Some weeks ago I checked
>>  and removed all duplicate code to use myfaces-commons-utils. So the 1.0.2
>>  release contains those classes taken from shared.
>> 
>>  regards,
>> 
>>  Leonardo Uribe
>> 
>>>  Regards,
>>>  Jakob
>>> 
>>>  [1] https://svn.apache.org/repos/asf/myfaces/common-shades/
>>> 
>>>  2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>>  Hi!
>>>>  While working on the mafyces-commons cleanup I figured that we have 
> tons of
>>>>  duplicated code spread over MyFaces.
>>>> 
>>>> 
>>>>  As an example I like to mention myfaces-commons-resourcehandler. 
> There are
>>>>  43 classes in total, and 35 of them are just 1:1 copied from other 
> projects
>>>>  to provide resource management, zip, etc. For me this is an 
> absolute no-go.
>>>>  Those classes have neither tests nor any documentation where they 
> got forked
>>>>  from. Nor will any bug which gets fixed in another module make 
> it's way over
>>>>  to all the other projects containing that very forked code. 
> That's just ...
>>>>  unbelievable unmaintainable.
>>>> 
>>>>  There are 2 different ways to solve this (depending on the 
> problem):
>>>> 
>>>>  A.) drop the functionality and provide a generalized solution. The 
> GZIP of
>>>>  myfaces-commons-resourcehandleris an obvious example:
>>>>  We now copy this code over the 4th time or even more. Instead of 
> doing this,
>>>>  we should rather do it in the classic unix fashion: do one thing, 
> but do it
>>>>  well.
>>>>  Which means I'd rather see all the GZIP stuff factored out into 
> an own
>>>>  mf-commons module as a Servlet Filter. This can then get applied to 
> what
>>>>  ever other mechanism you like. This could also (commonly) cover 
> cases like
>>>>  detecting http UserAgents which are not able to handle zipped 
> resources,
>>>>  etc. That way we could provide this logic ONCE and have complete 
> freedom
>>>>  over the configuration.
>>>> 
>>>>  B.) code reusable components once and use them in other projects 
> (ev via
>>>>  shading it in).
>>>>  ClassLoaderResourceLoader would be a perfect candidate! I grepped 
> through
>>>>  only the few pits which I have checked out locally and found this 
> class 7
>>>>  SEVEN times! I just can't believe that we can't move this 
> stuff to a shared
>>>>  modul...
>>>> 
>>>>  Same for FacesServletMapping. 6 times copied around,
>>>>  WebConfigProviderFactory 5 times, ...
>>>>  There are whole packages with 10++ classes which got copied 1:1!
>>>> 
>>>>  I really could cry seeing this :(
>>>> 
>>>> 
>>>>  What can we do to solve this?
>>>> 
>>>>  Theoretically myfaces-shared should contain this stuff. This is 
> exactly what
>>>>  it is for!
>>>>  Historically there have been some hand forged tweeks and ugly 
> hacks, but
>>>>  nowadays we have the maven-shade-plugin to make our live easier.
>>>> 
>>>>  So the suggestion is:
>>>> 
>>>>  1.) cleanup myfaces-shared. mf-shared has almost no checkstyle 
> rules
>>>>  applied.
>>>>  2.) add unit tests for myfaces-shared. Currently there are not 
> many...
>>>>  3.) move the shared code parts back to myfaces-shared and add unit 
> tests.
>>>>  4.) import myfaces-shared via maven dependency and use 
> <minimizeJar> and
>>>>  <relocations> to package the stuff
>>>> 
>>>>  [+1] fine go ahead (ideally: yes, what parts can I help with?)
>>>>  [0] dont care
>>>>  [-1] wont work because ...
>>>> 
>>>> 
>>>>  I've attached a file which contains all Classes which name 
> exists multiple
>>>>  times in MyFaces. The number is the cound how often they exist in 
> MyFaces. I
>>>>  excluded current20.
>>>>  Please note that classes with the same name do not necessarily have 
> the same
>>>>  content - but quite a lot actually do have! (scroll to the bottom 
> of the
>>>>  file ...)
>>>> 
>>>>  LieGrue,
>>>>  strub
>>>> 
>>> 
>>> 
>>> 
>>>  --
>>>  Jakob Korherr
>>> 
>>>  blog: http://www.jakobk.com
>>>  twitter: http://twitter.com/jakobkorherr
>>>  work: http://www.irian.at
>>> 
>> 
>

Re: [DISCUSS] how to get rid of tons of duplicated code

[Leonardo Uribe <lu...@gmail.com>]

Hi

Ok, let's check the proposal

MS>> So the suggestion is:
MS>>
MS>> 1.) cleanup myfaces-shared. mf-shared has almost no checkstyle
rules applied.

Yes, sounds good.

MS>> 2.) add unit tests for myfaces-shared. Currently there are not many...

Ok, sounds good too.

MS>> 3.) move the shared code parts back to myfaces-shared and add unit tests.

So, this means do one step back and move the code from myfaces-core
"shared" to myfaces-shared project? This breaks effectively the
changes done some months ago to make easier work with myfaces core
itself.

In that time the conclusion was: "core has priority over anything
else, so shared code must live in core, but myfaces-shared project
should just copy the code from there and have its own lifecycle"
(these are my own words as I understood).

So this point does not have practical sense, and go against everything
discussed earlier.

MS>> 4.) import myfaces-shared via maven dependency and use
<minimizeJar> and <relocations> to package the stuff

maven-shade-plugin is a good "tool" but doesn't fit well in this
scenario. The reason is we need an alternate release lifecycle for the
shared code between myfaces core and other projects.

Historically that was the very first intention behind myfaces-shared
project. Any myfaces core release requires some additional steps to do
(TCK), so that becomes a problem when you try to release other
libraries that depends of shared. So, to fix that, "shared" was
created, so the code can be released in a independent way, and prevent
myfaces core becomes an obstacle to release any other project
(tomahawk, portlet-bridge, ... ). So, to release tomahawk you release
shared first and then tomahawk.

maven-shade-plugin requires a released artifact to do its job. So, use
it impose that restriction. In "shared" case, preserve the original
intention becomes "imperative", and that's the reason why a goal was
created to copy the code from myfaces-core shared, so the release
manager can run this goal, commit the changes and then run a release.

My proposal in this case is do the same we did for shared, but for
"myfaces commons" case. Then we can use maven-shade-plugin in other
projects, but not over shared, instead over a released version of
myfaces-commons-utils. Keep tomahawk or portlet-bridge as is, using
shared project, because by its nature, those projects require classes
that are not meant to be used outside those cases.

Note do any hack in this part makes a little bit "obscure" how to make
changes, because everything becomes "centralized", but makes easier
maintain code. Additionally, we increase the risk of "side effects",
because a change done in core could introduce a bug in other parts. So
at the end this is a matter of how to keep our code "balanced", even
if some times it becomes a decision about "choose the less
inconvenient alternative".

regards,

Leonardo Uribe

2011/10/23 Leonardo Uribe <lu...@gmail.com>:
> Hi
>
> 2011/10/23 Jakob Korherr <ja...@gmail.com>:
>> Hi Mark,
>>
>> +1 - that's exactly what I have been trying to accomplish some time
>> ago (introducing common-shades [1]). Unfortunately, I was not
>> successful back then.
>>
>
> It is clear we need to "split" myfaces-impl into multiple modules. There
> are some parts that are useful for other projects. The code you did
> on commons-shade was the attempt to solve the problem of the
> duplicate code used on myfaces-test.
>
> Now the objective is find a way about how to reuse code in myfaces
> core between multiple projects effectively.
>
>> However, there is a slight problem with moving all this stuff into
>> MyFaces shared, which I want to point out: code size. If we really put
>> all the code that is shared across any MyFaces subproject into shared,
>> it will get fat and ugly (even more than it is right now). In
>> addition, if we continue including the whole shared project into
>> MyFaces core, MyFaces core impl will get bigger and bigger.
>>
>
> Yes, the problem basically is MyFaces shared does not have any order
> or any notion of API. There are code that is used only in tomahawk but not
> intended to use in any other place. There are some useful utitlities but
> sometimes without documentation, and there are some other code that is
> just obsolete. It it clear a cleanup of that location is
> necessary, but note priorities comes first, so this task has been delayed in
> order to deal with other important stuff. Now it is a good time to fix this.
>
>> Thus I'd like to suggest something similar which I wanted to
>> accomplish with common-shades: Introduce a new shared module, which
>> consists of many submodules that each handle a specific functionality
>> instead of being one fat module. With this approach each MyFaces
>> subproject would be able to pick out only the stuff it really needs.
>> Furthermore we would see more easily which code in shared is not used
>> anymore (I guess at the moment there is a lot of it), just by checking
>> which modules are still used in our poms.
>>
>
> That is the big question, how to split myfaces-impl and shared. Precisely
> the intention of myfaces-commons-utils projects was take the stuff that is
> useful from shared and build an usable API for developers outside MyFaces.
>
> For example, MyFaces HTML5 subproject was a good experiment to see
> which code is useful and should be added in a API. Some weeks ago I checked
> and removed all duplicate code to use myfaces-commons-utils. So the 1.0.2
> release contains those classes taken from shared.
>
> regards,
>
> Leonardo Uribe
>
>> Regards,
>> Jakob
>>
>> [1] https://svn.apache.org/repos/asf/myfaces/common-shades/
>>
>> 2011/10/23 Mark Struberg <st...@yahoo.de>:
>>> Hi!
>>> While working on the mafyces-commons cleanup I figured that we have tons of
>>> duplicated code spread over MyFaces.
>>>
>>>
>>> As an example I like to mention myfaces-commons-resourcehandler. There are
>>> 43 classes in total, and 35 of them are just 1:1 copied from other projects
>>> to provide resource management, zip, etc. For me this is an absolute no-go.
>>> Those classes have neither tests nor any documentation where they got forked
>>> from. Nor will any bug which gets fixed in another module make it's way over
>>> to all the other projects containing that very forked code. That's just ...
>>> unbelievable unmaintainable.
>>>
>>> There are 2 different ways to solve this (depending on the problem):
>>>
>>> A.) drop the functionality and provide a generalized solution. The GZIP of
>>> myfaces-commons-resourcehandleris an obvious example:
>>> We now copy this code over the 4th time or even more. Instead of doing this,
>>> we should rather do it in the classic unix fashion: do one thing, but do it
>>> well.
>>> Which means I'd rather see all the GZIP stuff factored out into an own
>>> mf-commons module as a Servlet Filter. This can then get applied to what
>>> ever other mechanism you like. This could also (commonly) cover cases like
>>> detecting http UserAgents which are not able to handle zipped resources,
>>> etc. That way we could provide this logic ONCE and have complete freedom
>>> over the configuration.
>>>
>>> B.) code reusable components once and use them in other projects (ev via
>>> shading it in).
>>> ClassLoaderResourceLoader would be a perfect candidate! I grepped through
>>> only the few pits which I have checked out locally and found this class 7
>>> SEVEN times! I just can't believe that we can't move this stuff to a shared
>>> modul...
>>>
>>> Same for FacesServletMapping. 6 times copied around,
>>> WebConfigProviderFactory 5 times, ...
>>> There are whole packages with 10++ classes which got copied 1:1!
>>>
>>> I really could cry seeing this :(
>>>
>>>
>>> What can we do to solve this?
>>>
>>> Theoretically myfaces-shared should contain this stuff. This is exactly what
>>> it is for!
>>> Historically there have been some hand forged tweeks and ugly hacks, but
>>> nowadays we have the maven-shade-plugin to make our live easier.
>>>
>>> So the suggestion is:
>>>
>>> 1.) cleanup myfaces-shared. mf-shared has almost no checkstyle rules
>>> applied.
>>> 2.) add unit tests for myfaces-shared. Currently there are not many...
>>> 3.) move the shared code parts back to myfaces-shared and add unit tests.
>>> 4.) import myfaces-shared via maven dependency and use <minimizeJar> and
>>> <relocations> to package the stuff
>>>
>>> [+1] fine go ahead (ideally: yes, what parts can I help with?)
>>> [0] dont care
>>> [-1] wont work because ...
>>>
>>>
>>> I've attached a file which contains all Classes which name exists multiple
>>> times in MyFaces. The number is the cound how often they exist in MyFaces. I
>>> excluded current20.
>>> Please note that classes with the same name do not necessarily have the same
>>> content - but quite a lot actually do have! (scroll to the bottom of the
>>> file ...)
>>>
>>> LieGrue,
>>> strub
>>>
>>
>>
>>
>> --
>> Jakob Korherr
>>
>> blog: http://www.jakobk.com
>> twitter: http://twitter.com/jakobkorherr
>> work: http://www.irian.at
>>
>

Re: [DISCUSS] how to get rid of tons of duplicated code

[Leonardo Uribe <lu...@gmail.com>]

Hi

2011/10/23 Jakob Korherr <ja...@gmail.com>:
> Hi Mark,
>
> +1 - that's exactly what I have been trying to accomplish some time
> ago (introducing common-shades [1]). Unfortunately, I was not
> successful back then.
>

It is clear we need to "split" myfaces-impl into multiple modules. There
are some parts that are useful for other projects. The code you did
on commons-shade was the attempt to solve the problem of the
duplicate code used on myfaces-test.

Now the objective is find a way about how to reuse code in myfaces
core between multiple projects effectively.

> However, there is a slight problem with moving all this stuff into
> MyFaces shared, which I want to point out: code size. If we really put
> all the code that is shared across any MyFaces subproject into shared,
> it will get fat and ugly (even more than it is right now). In
> addition, if we continue including the whole shared project into
> MyFaces core, MyFaces core impl will get bigger and bigger.
>

Yes, the problem basically is MyFaces shared does not have any order
or any notion of API. There are code that is used only in tomahawk but not
intended to use in any other place. There are some useful utitlities but
sometimes without documentation, and there are some other code that is
just obsolete. It it clear a cleanup of that location is
necessary, but note priorities comes first, so this task has been delayed in
order to deal with other important stuff. Now it is a good time to fix this.

> Thus I'd like to suggest something similar which I wanted to
> accomplish with common-shades: Introduce a new shared module, which
> consists of many submodules that each handle a specific functionality
> instead of being one fat module. With this approach each MyFaces
> subproject would be able to pick out only the stuff it really needs.
> Furthermore we would see more easily which code in shared is not used
> anymore (I guess at the moment there is a lot of it), just by checking
> which modules are still used in our poms.
>

That is the big question, how to split myfaces-impl and shared. Precisely
the intention of myfaces-commons-utils projects was take the stuff that is
useful from shared and build an usable API for developers outside MyFaces.

For example, MyFaces HTML5 subproject was a good experiment to see
which code is useful and should be added in a API. Some weeks ago I checked
and removed all duplicate code to use myfaces-commons-utils. So the 1.0.2
release contains those classes taken from shared.

regards,

Leonardo Uribe

> Regards,
> Jakob
>
> [1] https://svn.apache.org/repos/asf/myfaces/common-shades/
>
> 2011/10/23 Mark Struberg <st...@yahoo.de>:
>> Hi!
>> While working on the mafyces-commons cleanup I figured that we have tons of
>> duplicated code spread over MyFaces.
>>
>>
>> As an example I like to mention myfaces-commons-resourcehandler. There are
>> 43 classes in total, and 35 of them are just 1:1 copied from other projects
>> to provide resource management, zip, etc. For me this is an absolute no-go.
>> Those classes have neither tests nor any documentation where they got forked
>> from. Nor will any bug which gets fixed in another module make it's way over
>> to all the other projects containing that very forked code. That's just ...
>> unbelievable unmaintainable.
>>
>> There are 2 different ways to solve this (depending on the problem):
>>
>> A.) drop the functionality and provide a generalized solution. The GZIP of
>> myfaces-commons-resourcehandleris an obvious example:
>> We now copy this code over the 4th time or even more. Instead of doing this,
>> we should rather do it in the classic unix fashion: do one thing, but do it
>> well.
>> Which means I'd rather see all the GZIP stuff factored out into an own
>> mf-commons module as a Servlet Filter. This can then get applied to what
>> ever other mechanism you like. This could also (commonly) cover cases like
>> detecting http UserAgents which are not able to handle zipped resources,
>> etc. That way we could provide this logic ONCE and have complete freedom
>> over the configuration.
>>
>> B.) code reusable components once and use them in other projects (ev via
>> shading it in).
>> ClassLoaderResourceLoader would be a perfect candidate! I grepped through
>> only the few pits which I have checked out locally and found this class 7
>> SEVEN times! I just can't believe that we can't move this stuff to a shared
>> modul...
>>
>> Same for FacesServletMapping. 6 times copied around,
>> WebConfigProviderFactory 5 times, ...
>> There are whole packages with 10++ classes which got copied 1:1!
>>
>> I really could cry seeing this :(
>>
>>
>> What can we do to solve this?
>>
>> Theoretically myfaces-shared should contain this stuff. This is exactly what
>> it is for!
>> Historically there have been some hand forged tweeks and ugly hacks, but
>> nowadays we have the maven-shade-plugin to make our live easier.
>>
>> So the suggestion is:
>>
>> 1.) cleanup myfaces-shared. mf-shared has almost no checkstyle rules
>> applied.
>> 2.) add unit tests for myfaces-shared. Currently there are not many...
>> 3.) move the shared code parts back to myfaces-shared and add unit tests.
>> 4.) import myfaces-shared via maven dependency and use <minimizeJar> and
>> <relocations> to package the stuff
>>
>> [+1] fine go ahead (ideally: yes, what parts can I help with?)
>> [0] dont care
>> [-1] wont work because ...
>>
>>
>> I've attached a file which contains all Classes which name exists multiple
>> times in MyFaces. The number is the cound how often they exist in MyFaces. I
>> excluded current20.
>> Please note that classes with the same name do not necessarily have the same
>> content - but quite a lot actually do have! (scroll to the bottom of the
>> file ...)
>>
>> LieGrue,
>> strub
>>
>
>
>
> --
> Jakob Korherr
>
> blog: http://www.jakobk.com
> twitter: http://twitter.com/jakobkorherr
> work: http://www.irian.at
>

Re: [DISCUSS] how to get rid of tons of duplicated code

[Jakob Korherr <ja...@gmail.com>]

Aaaaah, nice! I did not know that. Great stuff :)

Regards,
Jakob

2011/10/23 Mark Struberg <st...@yahoo.de>:
>> With this approach each MyFaces
>> subproject would be able to pick out only the stuff it really needs.
>
> This is not needed if we use the maven-shade-plugin <minimizeJar> option [1] !
>
> With <minimizeJar> enabled the dependency classes are analyzed and only the classes which really got used will get shaded into the destination jar. The only limitation is when some classes get loaded via Class.forName() or similar. But as long as there is a bytecode invocation it will work smoothly.
>
> Thus I really see no reason why we cannot use maven-shared these days!
>
> Thus also a formal +1 from me.
>
> LieGrue,
> strub
>
>
>
> [1] http://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#minimizeJar
>
>
> ----- Original Message -----
>> From: Jakob Korherr <ja...@gmail.com>
>> To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg <st...@yahoo.de>
>> Cc:
>> Sent: Sunday, October 23, 2011 12:00 PM
>> Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
>>
>> Hi Mark,
>>
>> +1 - that's exactly what I have been trying to accomplish some time
>> ago (introducing common-shades [1]). Unfortunately, I was not
>> successful back then.
>>
>> However, there is a slight problem with moving all this stuff into
>> MyFaces shared, which I want to point out: code size. If we really put
>> all the code that is shared across any MyFaces subproject into shared,
>> it will get fat and ugly (even more than it is right now). In
>> addition, if we continue including the whole shared project into
>> MyFaces core, MyFaces core impl will get bigger and bigger.
>>
>> Thus I'd like to suggest something similar which I wanted to
>> accomplish with common-shades: Introduce a new shared module, which
>> consists of many submodules that each handle a specific functionality
>> instead of being one fat module. With this approach each MyFaces
>> subproject would be able to pick out only the stuff it really needs.
>> Furthermore we would see more easily which code in shared is not used
>> anymore (I guess at the moment there is a lot of it), just by checking
>> which modules are still used in our poms.
>>
>> Regards,
>> Jakob
>>
>> [1] https://svn.apache.org/repos/asf/myfaces/common-shades/
>>
>> 2011/10/23 Mark Struberg <st...@yahoo.de>:
>>>  Hi!
>>>  While working on the mafyces-commons cleanup I figured that we have tons of
>>>  duplicated code spread over MyFaces.
>>>
>>>
>>>  As an example I like to mention myfaces-commons-resourcehandler. There are
>>>  43 classes in total, and 35 of them are just 1:1 copied from other projects
>>>  to provide resource management, zip, etc. For me this is an absolute no-go.
>>>  Those classes have neither tests nor any documentation where they got
>> forked
>>>  from. Nor will any bug which gets fixed in another module make it's way
>> over
>>>  to all the other projects containing that very forked code. That's just
>> ...
>>>  unbelievable unmaintainable.
>>>
>>>  There are 2 different ways to solve this (depending on the problem):
>>>
>>>  A.) drop the functionality and provide a generalized solution. The GZIP of
>>>  myfaces-commons-resourcehandleris an obvious example:
>>>  We now copy this code over the 4th time or even more. Instead of doing
>> this,
>>>  we should rather do it in the classic unix fashion: do one thing, but do it
>>>  well.
>>>  Which means I'd rather see all the GZIP stuff factored out into an own
>>>  mf-commons module as a Servlet Filter. This can then get applied to what
>>>  ever other mechanism you like. This could also (commonly) cover cases like
>>>  detecting http UserAgents which are not able to handle zipped resources,
>>>  etc. That way we could provide this logic ONCE and have complete freedom
>>>  over the configuration.
>>>
>>>  B.) code reusable components once and use them in other projects (ev via
>>>  shading it in).
>>>  ClassLoaderResourceLoader would be a perfect candidate! I grepped through
>>>  only the few pits which I have checked out locally and found this class 7
>>>  SEVEN times! I just can't believe that we can't move this stuff to
>> a shared
>>>  modul...
>>>
>>>  Same for FacesServletMapping. 6 times copied around,
>>>  WebConfigProviderFactory 5 times, ...
>>>  There are whole packages with 10++ classes which got copied 1:1!
>>>
>>>  I really could cry seeing this :(
>>>
>>>
>>>  What can we do to solve this?
>>>
>>>  Theoretically myfaces-shared should contain this stuff. This is exactly
>> what
>>>  it is for!
>>>  Historically there have been some hand forged tweeks and ugly hacks, but
>>>  nowadays we have the maven-shade-plugin to make our live easier.
>>>
>>>  So the suggestion is:
>>>
>>>  1.) cleanup myfaces-shared. mf-shared has almost no checkstyle rules
>>>  applied.
>>>  2.) add unit tests for myfaces-shared. Currently there are not many...
>>>  3.) move the shared code parts back to myfaces-shared and add unit tests.
>>>  4.) import myfaces-shared via maven dependency and use <minimizeJar>
>> and
>>>  <relocations> to package the stuff
>>>
>>>  [+1] fine go ahead (ideally: yes, what parts can I help with?)
>>>  [0] dont care
>>>  [-1] wont work because ...
>>>
>>>
>>>  I've attached a file which contains all Classes which name exists
>> multiple
>>>  times in MyFaces. The number is the cound how often they exist in MyFaces.
>> I
>>>  excluded current20.
>>>  Please note that classes with the same name do not necessarily have the
>> same
>>>  content - but quite a lot actually do have! (scroll to the bottom of the
>>>  file ...)
>>>
>>>  LieGrue,
>>>  strub
>>>
>>
>>
>>
>> --
>> 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

Re: [DISCUSS] how to get rid of tons of duplicated code

[Mark Struberg <st...@yahoo.de>]

> With this approach each MyFaces
> subproject would be able to pick out only the stuff it really needs.

This is not needed if we use the maven-shade-plugin <minimizeJar> option [1] !

With <minimizeJar> enabled the dependency classes are analyzed and only the classes which really got used will get shaded into the destination jar. The only limitation is when some classes get loaded via Class.forName() or similar. But as long as there is a bytecode invocation it will work smoothly.

Thus I really see no reason why we cannot use maven-shared these days!

Thus also a formal +1 from me.

LieGrue,
strub



[1] http://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#minimizeJar


----- Original Message -----
> From: Jakob Korherr <ja...@gmail.com>
> To: MyFaces Development <de...@myfaces.apache.org>; Mark Struberg <st...@yahoo.de>
> Cc: 
> Sent: Sunday, October 23, 2011 12:00 PM
> Subject: Re: [DISCUSS] how to get rid of tons of duplicated code
> 
> Hi Mark,
> 
> +1 - that's exactly what I have been trying to accomplish some time
> ago (introducing common-shades [1]). Unfortunately, I was not
> successful back then.
> 
> However, there is a slight problem with moving all this stuff into
> MyFaces shared, which I want to point out: code size. If we really put
> all the code that is shared across any MyFaces subproject into shared,
> it will get fat and ugly (even more than it is right now). In
> addition, if we continue including the whole shared project into
> MyFaces core, MyFaces core impl will get bigger and bigger.
> 
> Thus I'd like to suggest something similar which I wanted to
> accomplish with common-shades: Introduce a new shared module, which
> consists of many submodules that each handle a specific functionality
> instead of being one fat module. With this approach each MyFaces
> subproject would be able to pick out only the stuff it really needs.
> Furthermore we would see more easily which code in shared is not used
> anymore (I guess at the moment there is a lot of it), just by checking
> which modules are still used in our poms.
> 
> Regards,
> Jakob
> 
> [1] https://svn.apache.org/repos/asf/myfaces/common-shades/
> 
> 2011/10/23 Mark Struberg <st...@yahoo.de>:
>>  Hi!
>>  While working on the mafyces-commons cleanup I figured that we have tons of
>>  duplicated code spread over MyFaces.
>> 
>> 
>>  As an example I like to mention myfaces-commons-resourcehandler. There are
>>  43 classes in total, and 35 of them are just 1:1 copied from other projects
>>  to provide resource management, zip, etc. For me this is an absolute no-go.
>>  Those classes have neither tests nor any documentation where they got 
> forked
>>  from. Nor will any bug which gets fixed in another module make it's way 
> over
>>  to all the other projects containing that very forked code. That's just 
> ...
>>  unbelievable unmaintainable.
>> 
>>  There are 2 different ways to solve this (depending on the problem):
>> 
>>  A.) drop the functionality and provide a generalized solution. The GZIP of
>>  myfaces-commons-resourcehandleris an obvious example:
>>  We now copy this code over the 4th time or even more. Instead of doing 
> this,
>>  we should rather do it in the classic unix fashion: do one thing, but do it
>>  well.
>>  Which means I'd rather see all the GZIP stuff factored out into an own
>>  mf-commons module as a Servlet Filter. This can then get applied to what
>>  ever other mechanism you like. This could also (commonly) cover cases like
>>  detecting http UserAgents which are not able to handle zipped resources,
>>  etc. That way we could provide this logic ONCE and have complete freedom
>>  over the configuration.
>> 
>>  B.) code reusable components once and use them in other projects (ev via
>>  shading it in).
>>  ClassLoaderResourceLoader would be a perfect candidate! I grepped through
>>  only the few pits which I have checked out locally and found this class 7
>>  SEVEN times! I just can't believe that we can't move this stuff to 
> a shared
>>  modul...
>> 
>>  Same for FacesServletMapping. 6 times copied around,
>>  WebConfigProviderFactory 5 times, ...
>>  There are whole packages with 10++ classes which got copied 1:1!
>> 
>>  I really could cry seeing this :(
>> 
>> 
>>  What can we do to solve this?
>> 
>>  Theoretically myfaces-shared should contain this stuff. This is exactly 
> what
>>  it is for!
>>  Historically there have been some hand forged tweeks and ugly hacks, but
>>  nowadays we have the maven-shade-plugin to make our live easier.
>> 
>>  So the suggestion is:
>> 
>>  1.) cleanup myfaces-shared. mf-shared has almost no checkstyle rules
>>  applied.
>>  2.) add unit tests for myfaces-shared. Currently there are not many...
>>  3.) move the shared code parts back to myfaces-shared and add unit tests.
>>  4.) import myfaces-shared via maven dependency and use <minimizeJar> 
> and
>>  <relocations> to package the stuff
>> 
>>  [+1] fine go ahead (ideally: yes, what parts can I help with?)
>>  [0] dont care
>>  [-1] wont work because ...
>> 
>> 
>>  I've attached a file which contains all Classes which name exists 
> multiple
>>  times in MyFaces. The number is the cound how often they exist in MyFaces. 
> I
>>  excluded current20.
>>  Please note that classes with the same name do not necessarily have the 
> same
>>  content - but quite a lot actually do have! (scroll to the bottom of the
>>  file ...)
>> 
>>  LieGrue,
>>  strub
>> 
> 
> 
> 
> -- 
> Jakob Korherr
> 
> blog: http://www.jakobk.com
> twitter: http://twitter.com/jakobkorherr
> work: http://www.irian.at
>

Re: [DISCUSS] how to get rid of tons of duplicated code

[Jakob Korherr <ja...@gmail.com>]

Hi Mark,

+1 - that's exactly what I have been trying to accomplish some time
ago (introducing common-shades [1]). Unfortunately, I was not
successful back then.

However, there is a slight problem with moving all this stuff into
MyFaces shared, which I want to point out: code size. If we really put
all the code that is shared across any MyFaces subproject into shared,
it will get fat and ugly (even more than it is right now). In
addition, if we continue including the whole shared project into
MyFaces core, MyFaces core impl will get bigger and bigger.

Thus I'd like to suggest something similar which I wanted to
accomplish with common-shades: Introduce a new shared module, which
consists of many submodules that each handle a specific functionality
instead of being one fat module. With this approach each MyFaces
subproject would be able to pick out only the stuff it really needs.
Furthermore we would see more easily which code in shared is not used
anymore (I guess at the moment there is a lot of it), just by checking
which modules are still used in our poms.

Regards,
Jakob

[1] https://svn.apache.org/repos/asf/myfaces/common-shades/

2011/10/23 Mark Struberg <st...@yahoo.de>:
> Hi!
> While working on the mafyces-commons cleanup I figured that we have tons of
> duplicated code spread over MyFaces.
>
>
> As an example I like to mention myfaces-commons-resourcehandler. There are
> 43 classes in total, and 35 of them are just 1:1 copied from other projects
> to provide resource management, zip, etc. For me this is an absolute no-go.
> Those classes have neither tests nor any documentation where they got forked
> from. Nor will any bug which gets fixed in another module make it's way over
> to all the other projects containing that very forked code. That's just ...
> unbelievable unmaintainable.
>
> There are 2 different ways to solve this (depending on the problem):
>
> A.) drop the functionality and provide a generalized solution. The GZIP of
> myfaces-commons-resourcehandleris an obvious example:
> We now copy this code over the 4th time or even more. Instead of doing this,
> we should rather do it in the classic unix fashion: do one thing, but do it
> well.
> Which means I'd rather see all the GZIP stuff factored out into an own
> mf-commons module as a Servlet Filter. This can then get applied to what
> ever other mechanism you like. This could also (commonly) cover cases like
> detecting http UserAgents which are not able to handle zipped resources,
> etc. That way we could provide this logic ONCE and have complete freedom
> over the configuration.
>
> B.) code reusable components once and use them in other projects (ev via
> shading it in).
> ClassLoaderResourceLoader would be a perfect candidate! I grepped through
> only the few pits which I have checked out locally and found this class 7
> SEVEN times! I just can't believe that we can't move this stuff to a shared
> modul...
>
> Same for FacesServletMapping. 6 times copied around,
> WebConfigProviderFactory 5 times, ...
> There are whole packages with 10++ classes which got copied 1:1!
>
> I really could cry seeing this :(
>
>
> What can we do to solve this?
>
> Theoretically myfaces-shared should contain this stuff. This is exactly what
> it is for!
> Historically there have been some hand forged tweeks and ugly hacks, but
> nowadays we have the maven-shade-plugin to make our live easier.
>
> So the suggestion is:
>
> 1.) cleanup myfaces-shared. mf-shared has almost no checkstyle rules
> applied.
> 2.) add unit tests for myfaces-shared. Currently there are not many...
> 3.) move the shared code parts back to myfaces-shared and add unit tests.
> 4.) import myfaces-shared via maven dependency and use <minimizeJar> and
> <relocations> to package the stuff
>
> [+1] fine go ahead (ideally: yes, what parts can I help with?)
> [0] dont care
> [-1] wont work because ...
>
>
> I've attached a file which contains all Classes which name exists multiple
> times in MyFaces. The number is the cound how often they exist in MyFaces. I
> excluded current20.
> Please note that classes with the same name do not necessarily have the same
> content - but quite a lot actually do have! (scroll to the bottom of the
> file ...)
>
> LieGrue,
> strub
>



-- 
Jakob Korherr

blog: http://www.jakobk.com
twitter: http://twitter.com/jakobkorherr
work: http://www.irian.at