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