yup, with respect to moving this change to 1.2 you'll get a +1 from me too ;)
LieGrue, strub --- Emmanuel Hugonnet <[email protected]> schrieb am Do, 5.2.2009: > Von: Emmanuel Hugonnet <[email protected]> > Betreff: Re: [mojo-dev] [Vote] Scmchangelog-maven-plugin > An: [email protected] > Datum: Donnerstag, 5. Februar 2009, 14:32 > Hi, > Yes I agree with you that I should have an adapter for each > scm with the > current adapter as a wrapper, the question I need to think > about is how > should I inject the correct adapter into the wrapper > (through the > constructor like the ScmManager and the grammar, or with a > static map). > Also I agree with you for the interface SvnGrammar and its > descendants which > should be renamed ScmGrammar etc. The same is to be applied > to SvnTargetEnum > and SvnLogEntry. > Adding Hg was more a proof of concept than a real > integration and it shows > through these misnamings ;o) I will put this into JIRA. > As it is cosmetic and add no functionnality I would rather > refactor for the > 1.2 version, pushing the current stable (as with not bug > entry left) into > production so current users can take advantage of the NPE > correction and the > filter parameter and I can get more feedbacks. > Thanks for your remarks. > Emmanuel > > > On Thu, Feb 5, 2009 at 12:09 PM, Mark Struberg > <[email protected]> wrote: > > > Hi Emmanuel! > > > > > > In general, I would prefer to have Hg and > Svn specific > > > code only in their > > > > own packages, using interfaces to access > them. I > > > personally don't think it's > > > > a good idea to have getSvnXxxx and getHgXxxx > functions > > > in one file. > > > > Otherwise I fear this will cause a problem > in > > > maintaining the code later if > > > > other scm providers will be added. > > > > > > You are right, but the ScmManager doesn't > provide such > > > facility, maybe this > > > should move to the provider. > > > > Sorry, I was not clear enough: I didn't mean to > move this code over to the > > ScmManager, but only to move those few bits which are > still mixed > > (getSvnListOfReleases, getHgListOfReleases) to the > already existing packages > > org.codehaus.mojo.scmchangelog.scm.hg and > > org.codehaus.mojo.scmchangelog.scm.svn > > and call them via interface or kind of SPI. > > > > LieGrue, > > strub > > > > > > --- Emmanuel Hugonnet <[email protected]> > schrieb am Do, 5.2.2009: > > > > > Von: Emmanuel Hugonnet > <[email protected]> > > > Betreff: Re: [mojo-dev] [Vote] > Scmchangelog-maven-plugin > > > An: [email protected] > > > Datum: Donnerstag, 5. Februar 2009, 11:54 > > > Hi, > > > > > > On Thu, Feb 5, 2009 at 11:02 AM, Mark Struberg > > > <[email protected]> wrote: > > > > > > > Hi! > > > > > > > > Sorry, but I'd give it a > > > > -1 > > > > because from only taking a glimpse at the > xref, the > > > code seems a bit mixed > > > > up, e.g. in > > > > > > > > protected List > ScmAdapter#getHgListOfReleases(.. > > > > > > > > 201 final ChangeLogScmResult logs = > > > this.manager.changeLog( > > > > repository, > > > > 202 fileSet, getScmVersion( > > > SvnTargetEnum.TRUNK, startRevision ), > > > > 203 getScmVersion( > SvnTargetEnum.TRUNK, > > > tag.getEndRevision() ), > > > > "" ); > > > > > > > > I personally don't like it much if > constants for > > > Svn are also used for > > > > Mercurial, etc. So maybe, you can simply > refactor out > > > the real scm provider > > > > independent constants (and functionality in > general) > > > and separate the > > > > concerns better? > > > > > > > > > Sorry, I think the Enum name came from the fact > that > > > historically this was a > > > svn only plugin, When we migrated to hg and add > potential > > > support to other > > > scms I did some refactoring but I may have missed > some > > > renaming. > > > > > > > > > > > In general, I would prefer to have Hg and > Svn specific > > > code only in their > > > > own packages, using interfaces to access > them. I > > > personally don't think it's > > > > a good idea to have getSvnXxxx and getHgXxxx > functions > > > in one file. > > > > Otherwise I fear this will cause a problem > in > > > maintaining the code later if > > > > other scm providers will be added. > > > > > > You are right, but the ScmManager doesn't > provide such > > > facility, maybe this > > > should move to the provider. > > > > > > > > > > > > > > > This brings me to the next very basic > question: Maybe > > > there is a > > > > fundamental reason and you already explained > it > > > (didn't follow the project > > > > closely), but is there a reason why you > don't use > > > the specialized > > > > ChangeLogConsumers provided by all the > > > maven-scm-providers? Is there any > > > > important information missing in the > ChangeLogSets > > > returned by? Or is the > > > > information inside the results that > different? > > > > If this information is really helpful or you > see a > > > chance to unify them, > > > > you may talk with Jason, Olivier Lamy and > Emanuel > > > Venisse to extend the > > > > scm-API? > > > > > > > > > Yes there is also a reason that is explained in > the for > > > developpers section. > > > First I was missing infos with the svn command if > I > > > didn't use xml (don't > > > remember much of it but it was my problem then) > and second > > > I need a > > > BetterChangeSet instead of the simple ChangeSet > from hg > > > where I can set the > > > revision number for the changeset. This info is > available > > > for most current > > > scms but it is not set by scm-api so I am losing > some info > > > there also. > > > That is why I had to override the scm-api. > > > Also I do som filtering in the hg tags consumer > and > > > SvnListConsumer which is > > > not available from the scm-api. > > > > > > > > > > > > > > txs and LieGrue, > > > > strub > > > > > > > > Emmanuel > > > > > > > > > > > > > > > --- Emmanuel Hugonnet > <[email protected]> > > > schrieb am Mi, 4.2.2009: > > > > > > > > > Von: Emmanuel Hugonnet > > > <[email protected]> > > > > > Betreff: [mojo-dev] [Vote] > > > Scmchangelog-maven-plugin > > > > > An: [email protected] > > > > > Datum: Mittwoch, 4. Februar 2009, 15:54 > > > > > Hi, > > > > > > > > > > We have been working on the > > > scmchangelog-maven-plugin and > > > > > we have closed all > > > > > opened issues. > > > > > > > > > > The snapshot is available at > > > > > * > > > > > > > > > > > > > > > http://snapshots.repository.codehaus.org/org/codehaus/mojo/scmchangelog-maven-plugin/1.1-SNAPSHOT > > > > > *< > > > > > > > > > > http://snapshots.repository.codehaus.org/org/codehaus/mojo/scmchangelog-maven-plugin/1.1-SNAPSHOT > > > > > > > > > > > > > > > The new site is available at > > > > > > > > > *http://mojo.codehaus.org/*scmchangelog-maven-plugin< > > > > > http://mojo.codehaus.org/scmchangelog-maven-plugin> > > > > > > > > > > The change log is available at > > > > > > > > > *http://mojo.codehaus.org/scmchangelog-maven-plugin/changelog.html*< > > > > > http://mojo.codehaus.org/rpm-maven-plugin> > > > > > > > > > > This vote will be open for 72 hours and > will use > > > lazy > > > > > consensus. > > > > > [+1] release it > > > > > [0] don't care > > > > > [-1] don't release! > > > > > > > > > > Thanks, > > > > > > > > > > Rémy & Emmanuel > > > > > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > > To unsubscribe from this list, please visit: > > > > > > > > http://xircles.codehaus.org/manage_email > > > > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > To unsubscribe from this list, please visit: > > > > http://xircles.codehaus.org/manage_email > > > > > > --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email
