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 » > > >
