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 »
