Create modularity for supporting multiple issue management systems

Dennis,

To save churn, I wonder if I could elicit more thought from you as to the shape of this? My first thought is an abstract class that stored an issue type map and then had some functions for the IssueAdapter to call. Does that seem reasonable?

I was thinking of having an Interface (or abstract class to start with) that holds the mappings. Perhaps we can make do with a simple Map object for it. That Map object would then be sent to the IssueAdapter from an (IMS specific) implementation of a report or announcement.

One of the problems with the current plugin design is that there is a lot of duplicate code between the Report mojo and the Announcement mojo, when it comes to parameters. The parameters often needs to be duplicated between the two mojos for every new IMS. What I would like is to have some kind of a configuration object that is specific to each IMS.

One way to do it would be to have something similar to the <archive> configuration used in the archiving plugins (JAR, WAR, EAR). That configuration is a Modello model which is reused between several plugins. Unfortunately that would require people to change their configuration for the Changes Plugin, but on the other hand we would get something that is much easier to maintain and extend.

Dennis,

The attached patch is a lot of changes that try to start the modularity you are looking for. This is complex enough that I'd like some feedback from you before committing; I don't want to have to back it out.

--benson

Note: this is a start. I didn't get involved in refactoring the unshared code.

To be perfectly honest, I'm not that enthralled with this plugin at all. But I'd like to use it, and I can't without some support for customized JIRAs. I'm willing to do some work to get those changes to the point where everyone is comfy, but not to take on a giant rewrite.

Benson,

Some thoughts and questions after a brief look at your patch.

• Lots of good stuff in there with the abstract class and all

• I haven't worked with enums before. What data is stored in an "instance" of IssueType?

• Is DefaultIssueManagmentSystem really needed? Or should it be renamed to JIRAIssueManagementSystem

• Was there a particular reason to change the logic in AnnouncementMojo? The reason I ask is that I envision changes.xml to be just another IMS in the future, and the changes to the logic seems to go against that. I.e it hard codes the changes + 1 that we currently have, making it harder to change that going forward.

A couple of more things I'd like to do:

• Add an interface IssueManagementSystem and use that wherever possible in place of AbstractIssueManagementSystem

• Add a constant in IssueAdapter:
  final String UNKNOWN_TYPE = "";

1. Enums. For each constant listed in the enum, there is one instance for the entire JVM. It has whatever fields and methods you declare on the enum. So, you are declaring a collection of named singletons which have a common contract.

2. Here's the modularity dilemma as I saw it. If we want to seriously support multiple IMSs, there is a lot of work to do. The comment at the top that we only support 'changes + 1' is there for a good reason given the code. So, I felt that I either had to tackle the big job of actually making it operate on and iterate over multiple systems, or I wanted to enforce the restriction. Now, I could scope the creation and use of the new object(s) to within the blocks for the individual IMSs, I realize, and leave things no worse off then they were. I'll do that.

3. If you prefer an interface, I think I'll rename the class from Abstract to Default, and add the interface. Unless you prefer the non-JIRA case to start out with no mappings, in which case I'd add the JIRA subclass.

Generally, I want to point out that this whole business is somewhat off to one side of the patch that started all this. All IMSes that I know are configurable. So, with or without this stuff, we still need to keep the original patch code that allows the user to configure their own mappings. I wonder if we should release what's currently checked in before continuing with any variation on this.

2. That'd be great.

3. I prefer to have an interface, and an abstract class with common implementation pieces. Also a JIRA subclass is better than a general purpose Default implementation, since the implementation in there is specific to JIRA.

The patch for this issue doesn't contradict your original patch to make the mappings configurable. IMO we should do both.

OK, here's the disconnect on the second part of (3). Before I got here, the code was just assuming that all IMSs used the same strings. And, for all I know, several IMSs do agree on those three strings. So, I preserved this. If you want to put those three mappings into JIRA and leave others empty until someone fills in correct mappings, I'll do that.

Yes, that's my preference, to keep things as separate as possible.

If we need the same mappings for Trac, then we should add a TracIMS class that we use for Trac. That way we are one step ahead of where we currently are: all supported IMSes have their own implementation and type mapping.

/Users/benson/asf/mvn/plugins/maven-changes-plugin svn log -r1134984
------------------------------------------------------------------------
r1134984 | bimargulies | 2011-06-12 17:08:34 -0400 (Sun, 12 Jun 2011) | 3 lines

MCHANGES-259 Start to build some modularity that knows that we have multiple
issue management systems at work in here. This is just the beginning.

------------------------------------------------------------------------
/Users/benson/asf/mvn/plugins/maven-changes-plugin