I've checked both the implementation and the unit tests, and I've tested it in my build system, and everything seems to be fine. Thanks a lot for the very fast turnaround time!!!
//P-J 2009/2/22 Ruben Willems <[email protected]> > Hi all > > > I committed the fix for the lastchangelabeller, > > @P-J > can you double check that the committed version is correct? > > just to be sure it is ok, the release is near ;-) > > > with kind regards > Ruben Willems > > > > On Thu, Feb 19, 2009 at 10:42 AM, Ruben Willems > <[email protected]>wrote: > >> Hi >> >> for the legal mumbo jumbo, you must explicitely say that you comply with >> the contributor license >> you can read it here >> >> >> http://confluence.public.thoughtworks.org/display/CCNET/Contributor+License+Agreement >> >> >> just mail to the devel list that you comply with it, and we can start >> integrating the patch >> >> with kind regards >> Ruben Willems >> >> On Thu, Feb 19, 2009 at 10:36 AM, P-J <[email protected]> wrote: >> >>> >>> I've submitted a patch to the ccnet-devel list as a reply to lukes >>> initial patch in the thread "Improving the LastChangeLabeler". >>> This is my first patch contribution, so I hope I got everything right, >>> and that the developers can integrate the patch into the trunk for the >>> 1.4.3 release. >>> >>> //P-J >>> >>> On 19 Feb, 08:23, Ruben Willems <[email protected]> wrote: >>> > Hi >>> > >>> > if you can make it this week, >>> > maybe we can squeeze it into the 1.4.3 release >>> > >>> > with kind regards >>> > Ruben Willems >>> > >>> > On Wed, Feb 18, 2009 at 5:00 PM, Per-Jonny Käck <[email protected]> >>> wrote: >>> > > Great, I think we have reached some kind of consensus now. >>> > > I will look into creating a new patch using the TortoiseSVN client. >>> > > The LastChangeLabeller behavior will be: >>> > >>> > > With prefix="pre" and allowDuplicateSubsequentLabels=false: >>> > > pre1234.1 >>> > > pre1234.2 >>> > > pre1235.1 >>> > > ->change configuration file and set prefix="new", then trigger a >>> forced >>> > > build without modifications >>> > > pre1235.2 >>> > > new1236.1 >>> > > new1236.2 >>> > >>> > > With prefix="pre" and allowDuplicateSubsequentLabels=true: >>> > > pre1234 >>> > > pre1234 >>> > > pre1235 >>> > > ->change configuration file and set prefix="new", then trigger a >>> forced >>> > > build without modifications >>> > > pre1235 >>> > > new1236 >>> > > new1236 >>> > >>> > > To minimize the impact on existing projects, >>> > > allowDuplicateSubsequentLabels=true will be the default behavior. >>> > >>> > > //P-J >>> > >>> > > 2009/2/18 [email protected] <[email protected]> >>> > >>> > >> Well, I'd like to thank you to you guys for bringing this back up >>> > >> again. I did find the patch for lastchangelabeller on the developers >>> > >> forum, but Im unsure of how to implement it. All I need it for >>> > >> lastchangelabeller to stop throwing an unknown if there is no >>> > >> transactions between builds. >>> > >> I think my requirements are similiar to Shauns....having it >>> increment >>> > >> with a xxx.1 xxx.2 is fine by me. >>> > >>> > >> On Feb 18, 6:51 am, Ruben Willems <[email protected]> wrote: >>> > >> > Hi... >>> > >>> > >> > read more » >>> > >>> > >> > just sparsely following this discussion >>> > >> > each labeler stands on its own, thre is no reletion between 2 >>> labellers. >>> > >>> > >> > the more the labellers act the same in a given situation, the >>> better of >>> > >> > course, >>> > >> > but this does not have to be. If there is a good reason, why not >>> do it. >>> > >>> > >> > there is a big change between the datalabeller and the default >>> labeller >>> > >> ;-) >>> > >>> > >> > Do keep in mind that when you make a breaking change, is not fun. >>> > >> > So try to keep the existing default behaviour if possible. >>> > >> > Adding extra properties is not a problem, as long as this doe snot >>> > >> result in >>> > >> > stuff like this >>> > >>> > >> > bool UseNewSetup >>> > >>> > >> > and than somewhere in code >>> > >>> > >> > if (UseNewSetup) >>> > >> > { >>> > >> > bla bla >>> > >>> > >> > } >>> > >>> > >> > in this case it would make more sense to create a new labeller >>> with the >>> > >> > desired behaviour >>> > >>> > >> > my 2 cents >>> > >>> > >> > with kind regards >>> > >> > Ruben Willems >>> > >>> > >> > On Wed, Feb 18, 2009 at 12:29 PM, Per-Jonny Käck < >>> [email protected]> >>> > >> wrote: >>> > >> > > I can agree with you that either forcing .nnn for all labels, or >>> > >> pre1234 >>> > >> > > followed by pre1234.2 would be more logical. >>> > >> > > It's just that the FileLabeller is implemented as pre1234 >>> followed by >>> > >> > > 1234-1. >>> > >> > > And lukes propsed patch seems to follow the same scheme. >>> > >> > > The question is if it is better have a consistent behavior >>> between the >>> > >> > > label implementations, or a logical implementation of the >>> > >> LastChangeLabeller >>> > >> > > only??? >>> > >> > > Is there any CruiseControl.NET developer that has an opinion? >>> > >>> > >> > > //P-J >>> > >>> > >> > > 2009/2/18 CinnamonDonkey <[email protected]> >>> > >>> > >> > >> Sounds good to me P-J, >>> > >>> > >> > >> The only comment I would have but it's just me being pedantic >>> now is >>> > >> > >> that if the labeller is allowing the post fix of '.nnn' then >>> all >>> > >> > >> labels should follow the same scheme: >>> > >>> > >> > >> pre1234.1 >>> > >> > >> pre1234.2 >>> > >> > >> pre1235.1 >>> > >>> > >> > >> ...and not: >>> > >>> > >> > >> pre1234 >>> > >> > >> pre1234.1 >>> > >> > >> pre1235 >>> > >>> > >> > >> ...as this does not seem logical to me. At the very least it >>> should >>> > >> > >> be: >>> > >>> > >> > >> pre1234 (<-- is effectively pre1234.1 as it was the >>> first >>> > >> > >> build with the label 1234) >>> > >> > >> pre1234.2 >>> > >>> > >> > >> Like I say, I'm just being pedantic. >>> > >>> > >> > >> Shaun ;-) >>> > >>> > >> > >> On 18 Feb, 10:48, P-J <[email protected]> wrote: >>> > >> > >> > Yes, we are talking about the same thing concerning duplicate >>> > >> labels. >>> > >>> > >> > >> > About my next paragaraph, it only applies if you have >>> specified a >>> > >> > >> > prefix. >>> > >> > >> > Let's say you are currently using the string "pre" as prefix, >>> and >>> > >> you >>> > >> > >> > have set AllowDuplicateSubsequentLabels=true for our new >>> > >> > >> > LastChangeLabeller. >>> > >> > >> > Using your examples, the labels would look like this: >>> > >> > >> > pre1234 >>> > >> > >> > pre1234 >>> > >> > >> > pre1235 >>> > >> > >> > At this point change your configuration file and set the >>> label >>> > >> prefix >>> > >> > >> > to the string "new", then force a new build without >>> modifications, >>> > >> and >>> > >> > >> > the labels will be as follows. >>> > >> > >> > pre1235 >>> > >> > >> > new1236 >>> > >> > >> > new1237 >>> > >>> > >> > >> > With AllowDuplicateSubsequentLabels=False the labels would >>> be: >>> > >> > >> > pre1234 >>> > >> > >> > pre1234.1 >>> > >> > >> > pre1235 >>> > >> > >> > At this point change your configuration file and set the >>> label >>> > >> prefix >>> > >> > >> > to the string "new", then force a new build without >>> modifications, >>> > >> and >>> > >> > >> > the labels will be as follows. >>> > >> > >> > pre1235.1 >>> > >> > >> > new1236 >>> > >> > >> > new1237 >>> > >>> > >> > >> > Personally I think this behavior would be ok, i.e. that the >>> change >>> > >> of >>> > >> > >> > the prefix will not be visible until you have a new build >>> with >>> > >> > >> > modifications. >>> > >> > >> > The good thing is about this solution is that it is easy to >>> use and >>> > >> > >> > understand, and doesn't restrict your choice of prefix in any >>> way. >>> > >>> > >> > >> > //P-J >>> > >>> > >> > >> > On 18 Feb, 11:29, CinnamonDonkey < >>> [email protected]> >>> > >> > >> > wrote: >>> > >>> > >> > >> > > Hi P-J, >>> > >>> > >> > >> > > Adding a new attribute, "AllowDuplicateSubsequentLabels" >>> sounds >>> > >> like a >>> > >> > >> > > good plan as it allows more flexibility to the end user and >>> > >> satisfies >>> > >> > >> > > my own requirements. >>> > >>> > >> > >> > > I'm not sure I fully understand the next paragraph, but I >>> think >>> > >> we are >>> > >> > >> > > talking about the same thing. >>> > >>> > >> > >> > > If I can end up with: >>> > >>> > >> > >> > > 1234 >>> > >> > >> > > 1234 >>> > >> > >> > > 1235 >>> > >> > >> > > 1235 >>> > >> > >> > > 1236 >>> > >> > >> > > 1237 >>> > >>> > >> > >> > > I'll be a very happy chappie, but I could live with: >>> > >>> > >> > >> > > 1234.1 >>> > >> > >> > > 1234.2 >>> > >> > >> > > 1235.1 >>> > >> > >> > > 1235.2 >>> > >> > >> > > 1236.1 >>> > >> > >> > > 1237.1 >>> > >>> > >> > >> > > If it was a necessary compromise but I think the post fix >>> should >>> > >> be >>> > >> > >> > > optional. >>> > >>> > >> > >> > > Shaun >>> > >>> > >> > >> > > On 18 Feb, 08:25, Per-Jonny Käck <[email protected]> wrote: >>> > >>> > >> > >> > > > Ok, then a "AllowDuplicateSubsequentLabels" attribute >>> (e.g. >>> > >> used by >>> > >> > >> the >>> > >> > >> > > > FileLabeller) should be added to lukes patch, since his >>> patch >>> > >> always >>> > >> > >> > > > increments the latest label. >>> > >>> > >> > >> > > > Do you have any problems with that if you change the >>> prefix in >>> > >> your >>> > >> > >> > > > configuration file, then you won't see the new prefix >>> until >>> > >> there is >>> > >> > >> a new >>> > >> > >> > > > build with modifications, since builds without >>> modifications >>> > >> will be >>> > >> > >> > > > labelled based on the last label. I think this behavior >>> would >>> > >> be ok, >>> > >> > >> and it >>> > >> > >> > > > makes the source code much cleaner. >>> > >>> > >> > >> > > > //P-J >>> > >>> > >> > >> > > > 2009/2/18 CinnamonDonkey <[email protected]> >>> > >>> > >> > >> > > > > Our requirement is that each build is labelled with the >>> > >> change >>> > >> > >> list >>> > >> > >> > > > > number (we use Perforce) used to create that build. >>> This >>> > >> means >>> > >> > >> that we >>> > >> > >> > > > > can relate any particular build directly back to a >>> specific >>> > >> point >>> > >> > >> in >>> > >> > >> > > > > the source history. Anything else is meaningless. >>> > >>> > >> > >> > > > > Since the change list numbers are unique an represent a >>> > >> particular >>> > >> > >> > > > > moment in time this should be perfectly possible. >>> > >>> > >> > >> > > > > - If a modification exists use the highest retrieved >>> > >> > >> modification >>> > >> > >> > > > > change list number. >>> > >> > >> > > > > - If no modifications exist, we must still be on the >>> same >>> > >> > >> > > > > modification change list number. >>> > >>> > >> > >> > > > > Duplicated labels should be valid as the build >>> date/time >>> > >> stamp is >>> > >> > >> used >>> > >> > >> > > > > to reflect the exact identity of that build. The label >>> > >> correlates >>> > >> > >> the >>> > >> > >> > > > > build with the source. >>> > >>> > >> > >> > > > > For us anyway (everyone's requirements are different >>> ;). >>> > >>> > >> > >> > > > > Regards, >>> > >> > >> > > > > Shaun >>> > >>> > >> > >> > > > > On 18 Feb, 08:04, P-J <[email protected]> wrote: >>> > >> > >> > > > > > This is still an issue on the latest build. >>> > >> > >> > > > > > The LastChangeLabeller is not good enough, and >>> requires a >>> > >> patch. >>> > >>> > >> > >> > > > > > The problem has also recently been mentioned e.g. in >>> the >>> > >> > >> following >>> > >> > >> > > > > > ccnet-user threads: >>> > >> > >> > > > > > "<LastChangelabeller> :Accurev/CCNET error: Given >>> > >> Update...". >>> > >> > >> > > > > > "LastChangeLabeller issue in case of a forced build >>> with no >>> > >> > >> changes in >>> > >> > >> > > > > > SVN" >>> > >> > >> > > > > > "LastChangeLabeller and Unknown with Accurev" >>> > >>> > >> > >> > > > > > A patch has been submitted by luke to ccnet-devel, >>> thread >>> > >> > >> "Improving >>> > >> > >> > > > > > the LastChangeLabeler". >>> > >> > >> > > > > > His patch appends a ".1" or increment an existing >>> integer >>> > >> to the >>> > >> > >> last >>> > >> > >> > > > > > label, instead of creating an "unkown" label. >>> > >> > >> > > > > > Is this good enough for everyone? In this patch: >>> > >> > >> > > > > > - If you change the Prefix in your config file, and >>> then >>> > >> make a >>> > >> > >> forced >>> > >> > >> > > > > > build with no modifications, then his patch will >>> return an >>> > >> > >> incremented >>> > >> > >> > > > > > label with the old prefix. Is this ok? >>> > >> > >> > > > > > - There is no "AllowDuplicateSubsequentLabels" to >>> choose if >>> > >> you >>> > >> > >> only >>> > >> > >> > > > > > want the LastChangeLabeller to return the latest >>> label, or >>> > >> if >>> > >> > >> you want >>> > >> > >> > > > > > it to return the latest label with an incremented >>> suffix. >>> > >>> > >> > >> > > > > > What do you want? >>> > >> > >> > > > > > I would be happy to contribute if could just get this >>> into >>> > >>> > ... >>> > >>> > läs mer » >> >> >> >
