ok, so I've just checked in a change which causes an eager parsing of the version string... if getVersion() is deprecated and we should avoid that this is a good step in that direction
(tests are ok now, those failures were only due to bad tests - invalid version numbers which didn't cause any problem till you call getVersion()) fabrizio On 8/6/07, Grégory Joseph <[email protected]> wrote: > Well, "later", we just won't have a String getVersion method. > Whatever constructs the beans from the xml will have to instanciate > Version/ModuleDefinition properly. > > g > > On Aug 6, 2007, at 15:54 , Fabrizio Giustina wrote: > > > what about moving the parsing to the constructor/setVersion() instead > > that repeating it each time getVersion() is called? > > Maybe not perfect (get/set should not throw exceptions) but a lot > > better than now... at least you get the error only while setting the > > module version initially, but if you pass that anybody can later call > > getVersion() without seeing its code explode for no clear reason ;) > > > > fabrizio > > > > > > On 8/6/07, Grégory Joseph <[email protected]> wrote: > >> > >> On Aug 6, 2007, at 15:44 , Fabrizio Giustina wrote: > >> > >>> Are you sure it's not better to display the version as parsed by > >>> magnolia? > >>> Maybe we should fix it so that a version parsing never throw an > >>> exception, or to parse it before... I think that displaying the > >>> "real" > >>> version used by magnolia is more useful than the original, unparsed > >>> string > >> > >> for one thing, it was breaking a test, so reverting this was a quick > >> fix. Otherwise I agree, but exception handling need to be thought of, > >> and right now I don't know. I don't think an invalid version number > >> should be accepted by the system. I'm already afraid that a $ > >> {project.version} slips in the system because of a badly configured > >> build... > >> > >> g > >> > >>> > >>> fabrizio > >>> > >>> > >>> On 8/6/07, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote: > >>>> > >>>> > >>>> Revision 10367 Author gjoseph Date 2007-08-06 15:40:06 +0200 > >>>> (Mon, 06 Aug > >>>> 2007) > >>>> Log Message // explicitely use the String version number to avoid > >>>> unnecessary parsing (and potential exceptions if said version > >>>> number is > >>>> invalid) > >>>> > >>>> Modified Paths > >>>> > >>>> magnolia/trunk/magnolia-core/src/main/java/info/magnolia/module/ > >>>> model/ModuleDefinition.java > >>>> > >>>> Diff > >>>> Modified: > >>>> magnolia/trunk/magnolia-core/src/main/java/info/magnolia/module/ > >>>> model/ModuleDefinition.java > >>>> (10366 => 10367) > >>>> --- > >>>> magnolia/trunk/magnolia-core/src/main/java/info/magnolia/module/ > >>>> model/ModuleDefinition.java > >>>> 2007-08-06 13:29:40 UTC (rev 10366) > >>>> +++ > >>>> magnolia/trunk/magnolia-core/src/main/java/info/magnolia/module/ > >>>> model/ModuleDefinition.java > >>>> 2007-08-06 13:40:06 UTC (rev 10367) > >>>> @@ -75,6 +75,7 @@ > >>>> } > >>>> > >>>> public String toString() { > >>>> - return getDisplayName() + " version " + getVersionDefinition(); > >>>> + // explicitely use the String version number to avoid > >>>> unnecessary parsing > >>>> (and potential exceptions if said version number is invalid) > >>>> + return getDisplayName() + " version " + getVersion(); > >>>> } > >>>> } > >>>> > >>>> > >>> > >>> ---------------------------------------------------------------- > >>> for list details see > >>> http://documentation.magnolia.info/docs/en/editor/stayupdated.html > >>> ---------------------------------------------------------------- > >> > >> > >> ---------------------------------------------------------------- > >> for list details see > >> http://documentation.magnolia.info/docs/en/editor/stayupdated.html > >> ---------------------------------------------------------------- > >> > > > > ---------------------------------------------------------------- > > for list details see > > http://documentation.magnolia.info/docs/en/editor/stayupdated.html > > ---------------------------------------------------------------- > > > ---------------------------------------------------------------- > for list details see > http://documentation.magnolia.info/docs/en/editor/stayupdated.html > ---------------------------------------------------------------- > ---------------------------------------------------------------- for list details see http://documentation.magnolia.info/docs/en/editor/stayupdated.html ----------------------------------------------------------------
