You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@continuum.apache.org by Emmanuel Venisse <em...@gmail.com> on 2008/06/24 11:25:05 UTC
Re: svn commit: r670751 - in /continuum/trunk: continuum-api/src/main/java/org/apache/maven/continuum/execution/ continuum-core/src/main/java/org/apache/maven/continuum/buildcontroller/ continuum-core/src/main/java/org/apache/maven/continuum/executio
On Tue, Jun 24, 2008 at 10:47 AM, Brett Porter <br...@apache.org> wrote:
> cool!
>
> one question, one nitpick:
>
> On 24/06/2008, at 4:59 AM, evenisse@apache.org wrote:
>
>
>> + // Check changes
>> + if ( !shouldBuild &&
>> !context.getScmResult().getChanges().isEmpty() )
>> + {
>> + try
>> + {
>> + ContinuumBuildExecutor executor =
>> buildExecutorManager.getBuildExecutor( project.getExecutorId() );
>> + shouldBuild = executor.shouldBuild(
>> context.getScmResult().getChanges(), project,
>> +
>> workingDirectoryService.getWorkingDirectory( project ),
>> +
>> context.getBuildDefinition() );
>> + }
>> + catch ( Exception e )
>> + {
>> + //nothing to do
>> + }
>> + }
>>
>
> it's not clear to me why the exception can be swallowed?
Fixed.
>
>
>> + int i = 0;
>> + while ( i <= files.size() - 1 )
>> + {
>> + ChangeFile file = files.get( i );
>> + boolean found = false;
>> + for ( String module : modules )
>> + {
>> + if ( file.getName().indexOf( module ) > 0 )
>> + {
>> + files.remove( file );
>> + found = true;
>> + break;
>> + }
>> + }
>> + if ( !found )
>> + {
>> + i++;
>> + }
>> + else
>> + {
>> + break;
>> + }
>> + }
>>
>
> can't the i stuff be a standard for loop?
What do you mean?
I can't use a 'for' loop for the i stuff because in this loop I remove some
file in the list.
Emmanuel
Re: svn commit: r670751 - in /continuum/trunk: continuum-api/src/main/java/org/apache/maven/continuum/execution/ continuum-core/src/main/java/org/apache/maven/continuum/buildcontroller/ continuum-core/src/main/java/org/apache/maven/continuum/executio
Posted by Emmanuel Venisse <em...@gmail.com>.
Brett, Olivier
All should be ok now.
Emmanuel
On Thu, Jun 26, 2008 at 11:26 AM, Olivier Lamy <ol...@apache.org> wrote:
> Hi,
> Just tested with rev 671587 and it looks forcing a build doesn't work
> anymore (forcing a build means building even if no scm changes).
>
> --
> Olivier
>
> 2008/6/26 Brett Porter <br...@apache.org>:
> >
> > On 25/06/2008, at 6:06 PM, Emmanuel Venisse wrote:
> >
> >>>>>> + int i = 0;
> >>>>>>>
> >>>>>>> + while ( i <= files.size() - 1 )
> >>>>>>> + {
> >>>>>>> + ChangeFile file = files.get( i );
> >>>>>>> + boolean found = false;
> >>>>>>> + for ( String module : modules )
> >>>>>>> + {
> >>>>>>> + if ( file.getName().indexOf( module ) > 0 )
> >>>>>>> + {
> >>>>>>> + files.remove( file );
> >>>>>>> + found = true;
> >>>>>>> + break;
> >>>>>>> + }
> >>>>>>> + }
> >>>>>>> + if ( !found )
> >>>>>>> + {
> >>>>>>> + i++;
> >>>>>>> + }
> >>>>>>> + else
> >>>>>>> + {
> >>>>>>> + break;
> >>>>>>> + }
> >>>>>>> + }
> >>>>>>>
> >>>>>>>
> >>>>>> can't the i stuff be a standard for loop?
> >>>>>>
> >>>>>
> >>>>>
> >>>>> What do you mean?
> >>>>> I can't use a 'for' loop for the i stuff because in this loop I
> remove
> >>>>> some
> >>>>> file in the list.
> >>>>>
> >>>>
> >>>> I think that's a bug then? It will break from both loops (found =
> true).
> >>>>
> >>>> You probably want to use an iterator, and then use i.remove()?
> >>>
> >>>
> >>> I didn't see a bug in all tests I done, I know I don't have writtten
> >>> junit
> >>> test and it's bad.
> >>> I'll look to refactor it with iterator
> >>>
> >>
> >> I fix an issue with my implementation but not in this part.
> >> Actual implementation works fine.
> >> I tested with an iterator but I get a concurrent modification exception
> so
> >> I
> >> think I'll let this part as is.
> >
> > concurrent modification exception using i.remove(), or using
> files.remove(
> > file )?
> >
> > I'm really confused, because AFAICT if found = true, both loops break
> anyway
> > right now.
> >
> > - Brett
> >
> > --
> > Brett Porter
> > brett@apache.org
> > http://blogs.exist.com/bporter/
> >
> >
>
Re: svn commit: r670751 - in /continuum/trunk: continuum-api/src/main/java/org/apache/maven/continuum/execution/ continuum-core/src/main/java/org/apache/maven/continuum/buildcontroller/ continuum-core/src/main/java/org/apache/maven/continuum/executio
Posted by Olivier Lamy <ol...@apache.org>.
Hi,
Just tested with rev 671587 and it looks forcing a build doesn't work
anymore (forcing a build means building even if no scm changes).
--
Olivier
2008/6/26 Brett Porter <br...@apache.org>:
>
> On 25/06/2008, at 6:06 PM, Emmanuel Venisse wrote:
>
>>>>>> + int i = 0;
>>>>>>>
>>>>>>> + while ( i <= files.size() - 1 )
>>>>>>> + {
>>>>>>> + ChangeFile file = files.get( i );
>>>>>>> + boolean found = false;
>>>>>>> + for ( String module : modules )
>>>>>>> + {
>>>>>>> + if ( file.getName().indexOf( module ) > 0 )
>>>>>>> + {
>>>>>>> + files.remove( file );
>>>>>>> + found = true;
>>>>>>> + break;
>>>>>>> + }
>>>>>>> + }
>>>>>>> + if ( !found )
>>>>>>> + {
>>>>>>> + i++;
>>>>>>> + }
>>>>>>> + else
>>>>>>> + {
>>>>>>> + break;
>>>>>>> + }
>>>>>>> + }
>>>>>>>
>>>>>>>
>>>>>> can't the i stuff be a standard for loop?
>>>>>>
>>>>>
>>>>>
>>>>> What do you mean?
>>>>> I can't use a 'for' loop for the i stuff because in this loop I remove
>>>>> some
>>>>> file in the list.
>>>>>
>>>>
>>>> I think that's a bug then? It will break from both loops (found = true).
>>>>
>>>> You probably want to use an iterator, and then use i.remove()?
>>>
>>>
>>> I didn't see a bug in all tests I done, I know I don't have writtten
>>> junit
>>> test and it's bad.
>>> I'll look to refactor it with iterator
>>>
>>
>> I fix an issue with my implementation but not in this part.
>> Actual implementation works fine.
>> I tested with an iterator but I get a concurrent modification exception so
>> I
>> think I'll let this part as is.
>
> concurrent modification exception using i.remove(), or using files.remove(
> file )?
>
> I'm really confused, because AFAICT if found = true, both loops break anyway
> right now.
>
> - Brett
>
> --
> Brett Porter
> brett@apache.org
> http://blogs.exist.com/bporter/
>
>
Re: svn commit: r670751 - in /continuum/trunk: continuum-api/src/main/java/org/apache/maven/continuum/execution/ continuum-core/src/main/java/org/apache/maven/continuum/buildcontroller/ continuum-core/src/main/java/org/apache/maven/continuum/executio
Posted by Brett Porter <br...@apache.org>.
On 25/06/2008, at 6:06 PM, Emmanuel Venisse wrote:
>>>>> + int i = 0;
>>>>>> + while ( i <= files.size() - 1 )
>>>>>> + {
>>>>>> + ChangeFile file = files.get( i );
>>>>>> + boolean found = false;
>>>>>> + for ( String module : modules )
>>>>>> + {
>>>>>> + if ( file.getName().indexOf( module ) > 0 )
>>>>>> + {
>>>>>> + files.remove( file );
>>>>>> + found = true;
>>>>>> + break;
>>>>>> + }
>>>>>> + }
>>>>>> + if ( !found )
>>>>>> + {
>>>>>> + i++;
>>>>>> + }
>>>>>> + else
>>>>>> + {
>>>>>> + break;
>>>>>> + }
>>>>>> + }
>>>>>>
>>>>>>
>>>>> can't the i stuff be a standard for loop?
>>>>>
>>>>
>>>>
>>>> What do you mean?
>>>> I can't use a 'for' loop for the i stuff because in this loop I
>>>> remove
>>>> some
>>>> file in the list.
>>>>
>>>
>>> I think that's a bug then? It will break from both loops (found =
>>> true).
>>>
>>> You probably want to use an iterator, and then use i.remove()?
>>
>>
>> I didn't see a bug in all tests I done, I know I don't have
>> writtten junit
>> test and it's bad.
>> I'll look to refactor it with iterator
>>
>
> I fix an issue with my implementation but not in this part.
> Actual implementation works fine.
> I tested with an iterator but I get a concurrent modification
> exception so I
> think I'll let this part as is.
concurrent modification exception using i.remove(), or using
files.remove( file )?
I'm really confused, because AFAICT if found = true, both loops break
anyway right now.
- Brett
--
Brett Porter
brett@apache.org
http://blogs.exist.com/bporter/
Re: svn commit: r670751 - in /continuum/trunk: continuum-api/src/main/java/org/apache/maven/continuum/execution/ continuum-core/src/main/java/org/apache/maven/continuum/buildcontroller/ continuum-core/src/main/java/org/apache/maven/continuum/executio
Posted by Emmanuel Venisse <em...@gmail.com>.
On Tue, Jun 24, 2008 at 4:33 PM, Emmanuel Venisse <
emmanuel.venisse@gmail.com> wrote:
>
>
> On Tue, Jun 24, 2008 at 3:51 PM, Brett Porter <br...@apache.org> wrote:
>
>>
>> On 24/06/2008, at 5:25 PM, Emmanuel Venisse wrote:
>>
>>
>>>
>>>
>>>>
>>>> + int i = 0;
>>>>> + while ( i <= files.size() - 1 )
>>>>> + {
>>>>> + ChangeFile file = files.get( i );
>>>>> + boolean found = false;
>>>>> + for ( String module : modules )
>>>>> + {
>>>>> + if ( file.getName().indexOf( module ) > 0 )
>>>>> + {
>>>>> + files.remove( file );
>>>>> + found = true;
>>>>> + break;
>>>>> + }
>>>>> + }
>>>>> + if ( !found )
>>>>> + {
>>>>> + i++;
>>>>> + }
>>>>> + else
>>>>> + {
>>>>> + break;
>>>>> + }
>>>>> + }
>>>>>
>>>>>
>>>> can't the i stuff be a standard for loop?
>>>>
>>>
>>>
>>> What do you mean?
>>> I can't use a 'for' loop for the i stuff because in this loop I remove
>>> some
>>> file in the list.
>>>
>>
>> I think that's a bug then? It will break from both loops (found = true).
>>
>> You probably want to use an iterator, and then use i.remove()?
>
>
> I didn't see a bug in all tests I done, I know I don't have writtten junit
> test and it's bad.
> I'll look to refactor it with iterator
>
I fix an issue with my implementation but not in this part.
Actual implementation works fine.
I tested with an iterator but I get a concurrent modification exception so I
think I'll let this part as is.
>
>
>>
>>
>> Cheers,
>> Brett
>>
>> --
>> Brett Porter
>> brett@apache.org
>> http://blogs.exist.com/bporter/
>>
>>
>
Re: svn commit: r670751 - in /continuum/trunk: continuum-api/src/main/java/org/apache/maven/continuum/execution/ continuum-core/src/main/java/org/apache/maven/continuum/buildcontroller/ continuum-core/src/main/java/org/apache/maven/continuum/executio
Posted by Emmanuel Venisse <em...@gmail.com>.
On Tue, Jun 24, 2008 at 3:51 PM, Brett Porter <br...@apache.org> wrote:
>
> On 24/06/2008, at 5:25 PM, Emmanuel Venisse wrote:
>
>
>>
>>
>>>
>>> + int i = 0;
>>>> + while ( i <= files.size() - 1 )
>>>> + {
>>>> + ChangeFile file = files.get( i );
>>>> + boolean found = false;
>>>> + for ( String module : modules )
>>>> + {
>>>> + if ( file.getName().indexOf( module ) > 0 )
>>>> + {
>>>> + files.remove( file );
>>>> + found = true;
>>>> + break;
>>>> + }
>>>> + }
>>>> + if ( !found )
>>>> + {
>>>> + i++;
>>>> + }
>>>> + else
>>>> + {
>>>> + break;
>>>> + }
>>>> + }
>>>>
>>>>
>>> can't the i stuff be a standard for loop?
>>>
>>
>>
>> What do you mean?
>> I can't use a 'for' loop for the i stuff because in this loop I remove
>> some
>> file in the list.
>>
>
> I think that's a bug then? It will break from both loops (found = true).
>
> You probably want to use an iterator, and then use i.remove()?
I didn't see a bug in all tests I done, I know I don't have writtten junit
test and it's bad.
I'll look to refactor it with iterator
>
>
> Cheers,
> Brett
>
> --
> Brett Porter
> brett@apache.org
> http://blogs.exist.com/bporter/
>
>
Re: svn commit: r670751 - in /continuum/trunk: continuum-api/src/main/java/org/apache/maven/continuum/execution/ continuum-core/src/main/java/org/apache/maven/continuum/buildcontroller/ continuum-core/src/main/java/org/apache/maven/continuum/executio
Posted by Brett Porter <br...@apache.org>.
On 24/06/2008, at 5:25 PM, Emmanuel Venisse wrote:
>
>
>>
>>
>>> + int i = 0;
>>> + while ( i <= files.size() - 1 )
>>> + {
>>> + ChangeFile file = files.get( i );
>>> + boolean found = false;
>>> + for ( String module : modules )
>>> + {
>>> + if ( file.getName().indexOf( module ) > 0 )
>>> + {
>>> + files.remove( file );
>>> + found = true;
>>> + break;
>>> + }
>>> + }
>>> + if ( !found )
>>> + {
>>> + i++;
>>> + }
>>> + else
>>> + {
>>> + break;
>>> + }
>>> + }
>>>
>>
>> can't the i stuff be a standard for loop?
>
>
> What do you mean?
> I can't use a 'for' loop for the i stuff because in this loop I
> remove some
> file in the list.
I think that's a bug then? It will break from both loops (found = true).
You probably want to use an iterator, and then use i.remove()?
Cheers,
Brett
--
Brett Porter
brett@apache.org
http://blogs.exist.com/bporter/