Uploaded image for project: 'zDev - M2E - WTP OSS'
  1. zDev - M2E - WTP OSS
  2. MECLIPSEWTP-187

Support for new war-overlay packaging type

        Details

        • Type: Task
        • Status: Open
        • Priority: Major
        • Resolution: Unresolved
        • Affects Version/s: None
        • Fix Version/s: None
        • Component/s: Overlay
        • Labels:
          None

          Description

          We have been looking into adding support for a new packaing type of war-overlay (see patch submitted on http://jira.codehaus.org/browse/MWAR-260) and have been attempting to replicate the functionality with m2eclipse-wtp.
          As part of the process and in order to aid our understanding of the overlay support we have refactored several classes and fixed some potential issues. Whilst this has resulted in larger changeset than we originally anticipated we feel changes are beneficial and we hope you will consider them. If you have any suggestions or concerns we'd be happy to rework the changes.
          Here is a brief summary of what we've done:
          Refactor of OverlayConfigurator to break into smaller methods.
          Moved filtering out into a generic set of classes for reuse. A single shared filter strategy is now used by all overlays.
          Reworked OverlayReferenceResolver delegating translation to/from URIs to new URIResolvers.
          Jar files are now lazily unpacked as and when JarEntries are accessed.
          Fixed a potential issue around the way jars are unpacked using a scheduled job.
          Removed caching as performance seemed adequate without.

          Please see the following branch to review the changes:
          https://github.com/SciSysUK/m2eclipse-wtp/tree/war-overlay

            Activity

            Hide
            Permalink
            fbricon Fred Bricon added a comment -

            That looks like great stuff Alex. Just a few observations right off the bat :

            • For a contribution that size, you and Phil will need to sign a CLA. BUT, we're in a grey area here, whether it's a Sonatype (since the code is hosted under their organization) or Red Hat (as the official maintainer) CLA needs to be determined. However, if you already signed the Sonatype CLA, I think you might be covered, but IANAL. I'll get back at you on that matter.
            • Honestly, I won't be able to review the code until a few weeks, right now I'm busy doing other stuff. But after browsing a bit through your source code, I noticed that :
              1- there are no licence headers, please use the Red Hat one (see https://github.com/sonatype/m2eclipse-wtp/blob/master/org.maven.ide.eclipse.wtp/src/org/maven/ide/eclipse/wtp/AppClientProjectConfiguratorDelegate.java)
              2- you should add some @author tags
              3- if the code you wrote supercedes my old crappy one, then delete it. We have a version control system that handles history pretty well.
            • I'm not comfortable adding support to a new packaging type that is not even sure to be accepted upstream, so again, I need to review the code to get a better picture.
            • review would probably be simpler for me if all use cases / features / bugfixes were cleanly separated in different commits or branches. If you can do anything about it, then great, if you can't, then I'll cope with it.

            Anyway, thank you and Phil very much for that contribution.

            Show
            fbricon Fred Bricon added a comment - That looks like great stuff Alex. Just a few observations right off the bat : For a contribution that size, you and Phil will need to sign a CLA. BUT, we're in a grey area here, whether it's a Sonatype (since the code is hosted under their organization) or Red Hat (as the official maintainer) CLA needs to be determined. However, if you already signed the Sonatype CLA, I think you might be covered, but IANAL. I'll get back at you on that matter. Honestly, I won't be able to review the code until a few weeks, right now I'm busy doing other stuff. But after browsing a bit through your source code, I noticed that : 1- there are no licence headers, please use the Red Hat one (see https://github.com/sonatype/m2eclipse-wtp/blob/master/org.maven.ide.eclipse.wtp/src/org/maven/ide/eclipse/wtp/AppClientProjectConfiguratorDelegate.java ) 2- you should add some @author tags 3- if the code you wrote supercedes my old crappy one, then delete it. We have a version control system that handles history pretty well. I'm not comfortable adding support to a new packaging type that is not even sure to be accepted upstream, so again, I need to review the code to get a better picture. review would probably be simpler for me if all use cases / features / bugfixes were cleanly separated in different commits or branches. If you can do anything about it, then great, if you can't, then I'll cope with it. Anyway, thank you and Phil very much for that contribution.

              People

              • Assignee:
                fbricon Fred Bricon
                Reporter:
                amc.kjg Alex Clarke
                Last Updated By:
                Fred Bricon
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Date of First Response: