Re: [infinispan-dev] optimistic locking - the time has come

2011-09-28 Thread Galder Zamarreño
Thanks Mircea. I've made some comments.

To all: 

If you're gonna submit a big pull req, please use the description to *summarise 
the changes* in order to help reviewers. This helps focus the attention of the 
reviewer to the pieces of code that are most relevant.

On Sep 27, 2011, at 7:10 PM, Mircea Markus wrote:

 Hi,
 
 I've issued a pull request for SPN-1131 (Add support for optimistic locking).
 My last run brought the same number of test failures in master and in my 
 branch, so might be a good idea to integrate it first and then continue 
 fixing tests. 
 
 Cheers,
 Mircea 
 
 
 ___
 infinispan-dev mailing list
 infinispan-dev@lists.jboss.org
 https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev


Re: [infinispan-dev] optimistic locking - the time has come

2011-09-28 Thread Manik Surtani

On 28 Sep 2011, at 10:05, Galder Zamarreño wrote:

 
 If you're gonna submit a big pull req, please use the description to 
 *summarise the changes* in order to help reviewers. This helps focus the 
 attention of the reviewer to the pieces of code that are most relevant.

+1!

--
Manik Surtani
ma...@jboss.org
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org



___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

Re: [infinispan-dev] optimistic locking - the time has come

2011-09-28 Thread Sanne Grinovero
Agreed, even better if we could use the commit description so it stays
with the code, and you can describe a step at a time.

We usually use a single-liner description, but for the more complex
patches we should add a paragraph. I did it occasionally, not sure
anybody noticed :)

Sanne

On 28 September 2011 11:27, Manik Surtani ma...@jboss.org wrote:

 On 28 Sep 2011, at 10:05, Galder Zamarreño wrote:

 If you're gonna submit a big pull req, please use the description to
 *summarise the changes* in order to help reviewers. This helps focus the
 attention of the reviewer to the pieces of code that are most relevant.

 +1!
 --
 Manik Surtani
 ma...@jboss.org
 twitter.com/maniksurtani
 Lead, Infinispan
 http://www.infinispan.org



 ___
 infinispan-dev mailing list
 infinispan-dev@lists.jboss.org
 https://lists.jboss.org/mailman/listinfo/infinispan-dev


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

Re: [infinispan-dev] optimistic locking - the time has come

2011-09-28 Thread Manik Surtani

On 28 Sep 2011, at 11:31, Sanne Grinovero wrote:

 Agreed, even better if we could use the commit description so it stays
 with the code, and you can describe a step at a time.
 
 We usually use a single-liner description, but for the more complex
 patches we should add a paragraph. I did it occasionally, not sure
 anybody noticed :)

I did notice - and I think that approach works really well.

--
Manik Surtani
ma...@jboss.org
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org



___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev


Re: [infinispan-dev] optimistic locking - the time has come

2011-09-28 Thread Mircea Markus

On 28 Sep 2011, at 10:05, Galder Zamarreño wrote:

 Thanks Mircea. I've made some comments.

Thanks, I've replied and updated the pull request.
Also added a summary: https://github.com/infinispan/infinispan/pull/551

 
 To all: 
 
 If you're gonna submit a big pull req, please use the description to 
 *summarise the changes* in order to help reviewers. This helps focus the 
 attention of the reviewer to the pieces of code that are most relevant.

+1

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

Re: [infinispan-dev] optimistic locking - the time has come

2011-09-28 Thread Mircea Markus

On 28 Sep 2011, at 15:02, Galder Zamarreño wrote:

 Thanks Mircea :)
 
 To all: I've had a look and made some comments, but this patch is definitely 
 worth another look by at least another person.
+1
Manik and Sanne are good candidates.


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev


[infinispan-dev] optimistic locking - the time has come

2011-09-27 Thread Mircea Markus
Hi,

I've issued a pull request for SPN-1131 (Add support for optimistic locking).
My last run brought the same number of test failures in master and in my 
branch, so might be a good idea to integrate it first and then continue fixing 
tests. 

Cheers,
Mircea 


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev