[pool] OK to commit?

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[pool] OK to commit?

Phil Steitz
I am working on a patch for POOL-361.  I think the analysis in the
ticket is correct and I think I know how to fix it (move validation into
create()).  I think I still have commit karma, but have not committed to
commons in quite a while.  Any objections to me committing directly to
[pool]?

Phil

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [pool] OK to commit?

Matt Sicker
I believe it’s CTR on Pool, so go for it.

On Sat, Jul 20, 2019 at 18:45, Phil Steitz <[hidden email]> wrote:

> I am working on a patch for POOL-361.  I think the analysis in the
> ticket is correct and I think I know how to fix it (move validation into
> create()).  I think I still have commit karma, but have not committed to
> commons in quite a while.  Any objections to me committing directly to
> [pool]?
>
> Phil
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
> --
Matt Sicker <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [pool] OK to commit?

James Carman
If you feel nervous, you can always push a branch or something and let
folks take a look.

On Sat, Jul 20, 2019 at 8:06 PM Matt Sicker <[hidden email]> wrote:

> I believe it’s CTR on Pool, so go for it.
>
> On Sat, Jul 20, 2019 at 18:45, Phil Steitz <[hidden email]> wrote:
>
> > I am working on a patch for POOL-361.  I think the analysis in the
> > ticket is correct and I think I know how to fix it (move validation into
> > create()).  I think I still have commit karma, but have not committed to
> > commons in quite a while.  Any objections to me committing directly to
> > [pool]?
> >
> > Phil
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> > --
> Matt Sicker <[hidden email]>
>
Reply | Threaded
Open this post in threaded view
|

Re: [pool] OK to commit?

garydgregory
In reply to this post by Phil Steitz
Go for it. As Matt pointed out, you can always create a PR and see have
Travis validate builds.

Gary

On Sat, Jul 20, 2019 at 7:45 PM Phil Steitz <[hidden email]> wrote:

> I am working on a patch for POOL-361.  I think the analysis in the
> ticket is correct and I think I know how to fix it (move validation into
> create()).  I think I still have commit karma, but have not committed to
> commons in quite a while.  Any objections to me committing directly to
> [pool]?
>
> Phil
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [pool] OK to commit?

Phil Steitz


On 7/20/19 7:23 PM, Gary Gregory wrote:
> Go for it. As Matt pointed out, you can always create a PR and see have
> Travis validate builds.

OK, I did the PR.  It has been a while since I have committed to [pool],
so some careful review would be most appreciated.  The intent should be
clear from the diffs - I just moved the validation into create() when
testOnCreate is enabled and tried to make sure that counters are
maintained.  I am not in love with the multiple returns in that method,
which I made slightly worse; but I could not see a simpler fix.

Phil

>
> Gary
>
> On Sat, Jul 20, 2019 at 7:45 PM Phil Steitz <[hidden email]> wrote:
>
>> I am working on a patch for POOL-361.  I think the analysis in the
>> ticket is correct and I think I know how to fix it (move validation into
>> create()).  I think I still have commit karma, but have not committed to
>> commons in quite a while.  Any objections to me committing directly to
>> [pool]?
>>
>> Phil
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]