myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Struberg <strub...@yahoo.de>
Subject Re: [DISCUSS] how to get rid of tons of duplicated code
Date Tue, 25 Oct 2011 11:33:08 GMT
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 <lu4242@gmail.com>
> To: MyFaces Development <dev@myfaces.apache.org>; Mark Struberg <struberg@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 <lu4242@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 <struberg@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 <lu4242@gmail.com>
>>>>  To: MyFaces Development <dev@myfaces.apache.org>; Mark 
> Struberg <struberg@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 <struberg@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 <lu4242@gmail.com>
>>>>>>   To: MyFaces Development <dev@myfaces.apache.org>; 
> Mark Struberg
>>>>  <struberg@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 <struberg@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 <lu4242@gmail.com>
>>>>>>>>    To: MyFaces Development 
> <dev@myfaces.apache.org>
>>>>>>>>    Cc: Mark Struberg <struberg@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 
> <lu4242@gmail.com>:
>>>>>>>>>     Hi
>>>>>>>>> 
>>>>>>>>>     2011/10/23 Jakob Korherr 
> <jakob.korherr@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 
> <struberg@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
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>

Mime
View raw message