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 06:38:37 GMT
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 <lu4242@gmail.com>
> To: MyFaces Development <dev@myfaces.apache.org>; Mark Struberg <struberg@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 <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