Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
42 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Adam Heath-2
[hidden email] wrote:

> Author: hansbak
> Date: Mon Jan 25 06:26:23 2010
> New Revision: 902716
>
> URL: http://svn.apache.org/viewvc?rev=902716&view=rev
> Log:
>
>  function leave feedback and screen
> - function auto relist item and screen
> - Get  feedback data from ebay site and save into ofbiz.
> - Add rate pictures into framework/images/webapp/images
> - screen for show awaiting feedback and recent feedback from buyer
> - help screens are provided

this was a bad commit, too many things done at once.  Please try to be
considerate of long-term maintainability, having small commits makes
it easier to verify and debug.
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

hans_bakker
Hi Adam,

comment accepted, i am trying to get my people to provide stuff in
smaller pieces. Not always easy but we try to do better.

Regards,
hans

On Mon, 2010-01-25 at 12:28 -0600, Adam Heath wrote:

> [hidden email] wrote:
> > Author: hansbak
> > Date: Mon Jan 25 06:26:23 2010
> > New Revision: 902716
> >
> > URL: http://svn.apache.org/viewvc?rev=902716&view=rev
> > Log:
> >
> >  function leave feedback and screen
> > - function auto relist item and screen
> > - Get  feedback data from ebay site and save into ofbiz.
> > - Add rate pictures into framework/images/webapp/images
> > - screen for show awaiting feedback and recent feedback from buyer
> > - help screens are provided
>
> this was a bad commit, too many things done at once.  Please try to be
> considerate of long-term maintainability, having small commits makes
> it easier to verify and debug.
--
Antwebsystems.com: Quality OFBiz services for competitive rates

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Tim Ruppert
As the committer, you can just not put them into OFBiz until they are of better quality Hans - that is your obligation and something that the rest of us abide to.  Please do better in the future - we know you're trying - but just because you've received it doesn't mean you have to commit it - you can just wait until it's in good enough shape to commit it ...

Cheers,
Ruppert
--
Tim Ruppert
HotWax Media
http://www.hotwaxmedia.com

o:801.649.6594
f:801.649.6595

On Jan 25, 2010, at 4:28 PM, Hans Bakker wrote:

> Hi Adam,
>
> comment accepted, i am trying to get my people to provide stuff in
> smaller pieces. Not always easy but we try to do better.
>
> Regards,
> hans
>
> On Mon, 2010-01-25 at 12:28 -0600, Adam Heath wrote:
>> [hidden email] wrote:
>>> Author: hansbak
>>> Date: Mon Jan 25 06:26:23 2010
>>> New Revision: 902716
>>>
>>> URL: http://svn.apache.org/viewvc?rev=902716&view=rev
>>> Log:
>>>
>>> function leave feedback and screen
>>> - function auto relist item and screen
>>> - Get  feedback data from ebay site and save into ofbiz.
>>> - Add rate pictures into framework/images/webapp/images
>>> - screen for show awaiting feedback and recent feedback from buyer
>>> - help screens are provided
>>
>> this was a bad commit, too many things done at once.  Please try to be
>> considerate of long-term maintainability, having small commits makes
>> it easier to verify and debug.
> --
> Antwebsystems.com: Quality OFBiz services for competitive rates
>


smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

David E. Jones-2

This seems to be a little bit condescending. Trying to pick another fight?

I'm just glad to have ring-side seats... ;)

-David


On Jan 25, 2010, at 9:39 PM, Tim Ruppert wrote:

> As the committer, you can just not put them into OFBiz until they are of better quality Hans - that is your obligation and something that the rest of us abide to.  Please do better in the future - we know you're trying - but just because you've received it doesn't mean you have to commit it - you can just wait until it's in good enough shape to commit it ...
>
> Cheers,
> Ruppert
> --
> Tim Ruppert
> HotWax Media
> http://www.hotwaxmedia.com
>
> o:801.649.6594
> f:801.649.6595
>
> On Jan 25, 2010, at 4:28 PM, Hans Bakker wrote:
>
>> Hi Adam,
>>
>> comment accepted, i am trying to get my people to provide stuff in
>> smaller pieces. Not always easy but we try to do better.
>>
>> Regards,
>> hans
>>
>> On Mon, 2010-01-25 at 12:28 -0600, Adam Heath wrote:
>>> [hidden email] wrote:
>>>> Author: hansbak
>>>> Date: Mon Jan 25 06:26:23 2010
>>>> New Revision: 902716
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=902716&view=rev
>>>> Log:
>>>>
>>>> function leave feedback and screen
>>>> - function auto relist item and screen
>>>> - Get  feedback data from ebay site and save into ofbiz.
>>>> - Add rate pictures into framework/images/webapp/images
>>>> - screen for show awaiting feedback and recent feedback from buyer
>>>> - help screens are provided
>>>
>>> this was a bad commit, too many things done at once.  Please try to be
>>> considerate of long-term maintainability, having small commits makes
>>> it easier to verify and debug.
>> --
>> Antwebsystems.com: Quality OFBiz services for competitive rates
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Adam Heath-2
David E Jones wrote:
> This seems to be a little bit condescending. Trying to pick another fight?
>
> I'm just glad to have ring-side seats... ;)

I agree with you, David.
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Tim Ruppert
In reply to this post by David E. Jones-2
Not trying to - but sorry for the tone if it came across that way Hans.  I'm simply asking why it gets past the committer's desk as "I'm trying to get my people to provide smaller pieces" is an ok way of committing code.  Isn't that the responsibility of the committer to tell the person that's providing the code to go back to the drawing board and break it up?  Isn't that our job as committers?  And as the rest of the committers, isn't it our job to remind the offending committer to spend more time with the person who's providing the code so that we don't have to dig thru this much mess?

It's not always easy for any of us, but I just don't see this from other people, so I wanted to remind Hans.  If all the other committers think this is ok (which obviously they didn't since Adam brought it up), then I'll happily back off.  Since that's not the case - and this obviously doesn't follow the best practices of the project - please let me know how I should encourage Hans to do what is clearly in our best practices.

What should be done is that this mess of a commit should be reverted and put back in in pieces - you have to start somewhere and sometimes it's just not good enough when you have an entire community to just say you're trying hard.  My two cents.

Cheers,
Ruppert
--
Tim Ruppert
HotWax Media
http://www.hotwaxmedia.com

o:801.649.6594
f:801.649.6595

On Jan 25, 2010, at 8:47 PM, David E Jones wrote:

>
> This seems to be a little bit condescending. Trying to pick another fight?
>
> I'm just glad to have ring-side seats... ;)
>
> -David
>
>
> On Jan 25, 2010, at 9:39 PM, Tim Ruppert wrote:
>
>> As the committer, you can just not put them into OFBiz until they are of better quality Hans - that is your obligation and something that the rest of us abide to.  Please do better in the future - we know you're trying - but just because you've received it doesn't mean you have to commit it - you can just wait until it's in good enough shape to commit it ...
>>
>> Cheers,
>> Ruppert
>> --
>> Tim Ruppert
>> HotWax Media
>> http://www.hotwaxmedia.com
>>
>> o:801.649.6594
>> f:801.649.6595
>>
>> On Jan 25, 2010, at 4:28 PM, Hans Bakker wrote:
>>
>>> Hi Adam,
>>>
>>> comment accepted, i am trying to get my people to provide stuff in
>>> smaller pieces. Not always easy but we try to do better.
>>>
>>> Regards,
>>> hans
>>>
>>> On Mon, 2010-01-25 at 12:28 -0600, Adam Heath wrote:
>>>> [hidden email] wrote:
>>>>> Author: hansbak
>>>>> Date: Mon Jan 25 06:26:23 2010
>>>>> New Revision: 902716
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=902716&view=rev
>>>>> Log:
>>>>>
>>>>> function leave feedback and screen
>>>>> - function auto relist item and screen
>>>>> - Get  feedback data from ebay site and save into ofbiz.
>>>>> - Add rate pictures into framework/images/webapp/images
>>>>> - screen for show awaiting feedback and recent feedback from buyer
>>>>> - help screens are provided
>>>>
>>>> this was a bad commit, too many things done at once.  Please try to be
>>>> considerate of long-term maintainability, having small commits makes
>>>> it easier to verify and debug.
>>> --
>>> Antwebsystems.com: Quality OFBiz services for competitive rates
>>>
>>
>


smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Erwan de FERRIERES-3


Le 26/01/2010 15:28, Tim Ruppert a écrit :

> Not trying to - but sorry for the tone if it came across that way Hans.  I'm simply asking why it gets past the committer's desk as "I'm trying to get my people to provide smaller pieces" is an ok way of committing code.  Isn't that the responsibility of the committer to tell the person that's providing the code to go back to the drawing board and break it up?  Isn't that our job as committers?  And as the rest of the committers, isn't it our job to remind the offending committer to spend more time with the person who's providing the code so that we don't have to dig thru this much mess?
>
> It's not always easy for any of us, but I just don't see this from other people, so I wanted to remind Hans.  If all the other committers think this is ok (which obviously they didn't since Adam brought it up), then I'll happily back off.  Since that's not the case - and this obviously doesn't follow the best practices of the project - please let me know how I should encourage Hans to do what is clearly in our best practices.
>
> What should be done is that this mess of a commit should be reverted and put back in in pieces - you have to start somewhere and sometimes it's just not good enough when you have an entire community to just say you're trying hard.  My two cents.
>
> Cheers,
> Ruppert
> --
> Tim Ruppert
> HotWax Media
> http://www.hotwaxmedia.com
>
> o:801.649.6594
> f:801.649.6595
>

The problem we have here, is that a too big commit is not well reviewed.
 From what I saw, and what has been reverted, it seems that a lot of
lines have not been validated.
We found (at Nereide) already one service going from https to http, and
the other thing is a suppression of all the shipment methods from the
demo data.

As we are all on different timezones, the responses and explanations are
a bit slow, and we cannot understand the reason of those changes.

I'm sure that smaller commits will help us understand the work done, and
the reason of the changes, and also point easily the reason of it.

This were my 2 cents....

--
Erwan de FERRIERES
www.nereide.biz
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/

Jacques Le Roux
Administrator
In reply to this post by Tim Ruppert
I can't disagree. It's good to see more things in OFBiz, having them in a better state should certainly be researched. I'm confident
in Hans to tackle this with time, we have already seen clear progresses.
But please Hans, even if it's the same work for review, break these big patches in smaller pieces. If we need to revert something it
will be much easier, thanks.

My 2cts

Jacques

From: "Tim Ruppert" <[hidden email]>
Not trying to - but sorry for the tone if it came across that way Hans.  I'm simply asking why it gets past the committer's desk as
"I'm trying to get my people to provide smaller pieces" is an ok way of committing code.  Isn't that the responsibility of the
committer to tell the person that's providing the code to go back to the drawing board and break it up?  Isn't that our job as
committers?  And as the rest of the committers, isn't it our job to remind the offending committer to spend more time with the
person who's providing the code so that we don't have to dig thru this much mess?

It's not always easy for any of us, but I just don't see this from other people, so I wanted to remind Hans.  If all the other
committers think this is ok (which obviously they didn't since Adam brought it up), then I'll happily back off.  Since that's not
the case - and this obviously doesn't follow the best practices of the project - please let me know how I should encourage Hans to
do what is clearly in our best practices.

What should be done is that this mess of a commit should be reverted and put back in in pieces - you have to start somewhere and
sometimes it's just not good enough when you have an entire community to just say you're trying hard.  My two cents.

Cheers,
Ruppert
--
Tim Ruppert
HotWax Media
http://www.hotwaxmedia.com

o:801.649.6594
f:801.649.6595

On Jan 25, 2010, at 8:47 PM, David E Jones wrote:

>
> This seems to be a little bit condescending. Trying to pick another fight?
>
> I'm just glad to have ring-side seats... ;)
>
> -David
>
>
> On Jan 25, 2010, at 9:39 PM, Tim Ruppert wrote:
>
>> As the committer, you can just not put them into OFBiz until they are of better quality Hans - that is your obligation and
>> something that the rest of us abide to.  Please do better in the future - we know you're trying - but just because you've
>> received it doesn't mean you have to commit it - you can just wait until it's in good enough shape to commit it ...
>>
>> Cheers,
>> Ruppert
>> --
>> Tim Ruppert
>> HotWax Media
>> http://www.hotwaxmedia.com
>>
>> o:801.649.6594
>> f:801.649.6595
>>
>> On Jan 25, 2010, at 4:28 PM, Hans Bakker wrote:
>>
>>> Hi Adam,
>>>
>>> comment accepted, i am trying to get my people to provide stuff in
>>> smaller pieces. Not always easy but we try to do better.
>>>
>>> Regards,
>>> hans
>>>
>>> On Mon, 2010-01-25 at 12:28 -0600, Adam Heath wrote:
>>>> [hidden email] wrote:
>>>>> Author: hansbak
>>>>> Date: Mon Jan 25 06:26:23 2010
>>>>> New Revision: 902716
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=902716&view=rev
>>>>> Log:
>>>>>
>>>>> function leave feedback and screen
>>>>> - function auto relist item and screen
>>>>> - Get  feedback data from ebay site and save into ofbiz.
>>>>> - Add rate pictures into framework/images/webapp/images
>>>>> - screen for show awaiting feedback and recent feedback from buyer
>>>>> - help screens are provided
>>>>
>>>> this was a bad commit, too many things done at once.  Please try to be
>>>> considerate of long-term maintainability, having small commits makes
>>>> it easier to verify and debug.
>>> --
>>> Antwebsystems.com: Quality OFBiz services for competitive rates
>>>
>>
>



Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Adrian Crum
In reply to this post by Tim Ruppert
Tim,

I didn't take your remark as condescending - in fact I agree with you.
It isn't okay for any committer to simply pass the buck (or blame).

If Hans seems to be a frequent target for commit remarks it is because
he earned it. There have been too many times when he has checked in code
that was written by beginner programmers without reviewing it first. Or
he has changed code without understanding it first. Those behaviors have
had a detrimental effect on the project.

Please don't stop pushing for code quality and responsible commits. You
have my support.

-Adrian


Tim Ruppert wrote:

> Not trying to - but sorry for the tone if it came across that way Hans.  I'm simply asking why it gets past the committer's desk as "I'm trying to get my people to provide smaller pieces" is an ok way of committing code.  Isn't that the responsibility of the committer to tell the person that's providing the code to go back to the drawing board and break it up?  Isn't that our job as committers?  And as the rest of the committers, isn't it our job to remind the offending committer to spend more time with the person who's providing the code so that we don't have to dig thru this much mess?
>
> It's not always easy for any of us, but I just don't see this from other people, so I wanted to remind Hans.  If all the other committers think this is ok (which obviously they didn't since Adam brought it up), then I'll happily back off.  Since that's not the case - and this obviously doesn't follow the best practices of the project - please let me know how I should encourage Hans to do what is clearly in our best practices.
>
> What should be done is that this mess of a commit should be reverted and put back in in pieces - you have to start somewhere and sometimes it's just not good enough when you have an entire community to just say you're trying hard.  My two cents.
>
> Cheers,
> Ruppert
> --
> Tim Ruppert
> HotWax Media
> http://www.hotwaxmedia.com
>
> o:801.649.6594
> f:801.649.6595
>
> On Jan 25, 2010, at 8:47 PM, David E Jones wrote:
>
>> This seems to be a little bit condescending. Trying to pick another fight?
>>
>> I'm just glad to have ring-side seats... ;)
>>
>> -David
>>
>>
>> On Jan 25, 2010, at 9:39 PM, Tim Ruppert wrote:
>>
>>> As the committer, you can just not put them into OFBiz until they are of better quality Hans - that is your obligation and something that the rest of us abide to.  Please do better in the future - we know you're trying - but just because you've received it doesn't mean you have to commit it - you can just wait until it's in good enough shape to commit it ...
>>>
>>> Cheers,
>>> Ruppert
>>> --
>>> Tim Ruppert
>>> HotWax Media
>>> http://www.hotwaxmedia.com
>>>
>>> o:801.649.6594
>>> f:801.649.6595
>>>
>>> On Jan 25, 2010, at 4:28 PM, Hans Bakker wrote:
>>>
>>>> Hi Adam,
>>>>
>>>> comment accepted, i am trying to get my people to provide stuff in
>>>> smaller pieces. Not always easy but we try to do better.
>>>>
>>>> Regards,
>>>> hans
>>>>
>>>> On Mon, 2010-01-25 at 12:28 -0600, Adam Heath wrote:
>>>>> [hidden email] wrote:
>>>>>> Author: hansbak
>>>>>> Date: Mon Jan 25 06:26:23 2010
>>>>>> New Revision: 902716
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=902716&view=rev
>>>>>> Log:
>>>>>>
>>>>>> function leave feedback and screen
>>>>>> - function auto relist item and screen
>>>>>> - Get  feedback data from ebay site and save into ofbiz.
>>>>>> - Add rate pictures into framework/images/webapp/images
>>>>>> - screen for show awaiting feedback and recent feedback from buyer
>>>>>> - help screens are provided
>>>>> this was a bad commit, too many things done at once.  Please try to be
>>>>> considerate of long-term maintainability, having small commits makes
>>>>> it easier to verify and debug.
>>>> --
>>>> Antwebsystems.com: Quality OFBiz services for competitive rates
>>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Jacopo Cappellato-4
I understand the concerns and remarks but please, let's try to stay focused on specific details (that needs to be fixed): only in this way the conversation will be useful and we will help each other to get better code.
If instead we make general assumptions on other's work (like defining the effort of a committer "a mess" or the effort of a "beginner programmer") we will just end up with people being offended and defensive.

"make code not war" :-)

Jacopo

On Jan 26, 2010, at 4:44 PM, Adrian Crum wrote:

> Tim,
>
> I didn't take your remark as condescending - in fact I agree with you. It isn't okay for any committer to simply pass the buck (or blame).
>
> If Hans seems to be a frequent target for commit remarks it is because he earned it. There have been too many times when he has checked in code that was written by beginner programmers without reviewing it first. Or he has changed code without understanding it first. Those behaviors have had a detrimental effect on the project.
>
> Please don't stop pushing for code quality and responsible commits. You have my support.
>
> -Adrian
>
>
> Tim Ruppert wrote:
>> Not trying to - but sorry for the tone if it came across that way Hans.  I'm simply asking why it gets past the committer's desk as "I'm trying to get my people to provide smaller pieces" is an ok way of committing code.  Isn't that the responsibility of the committer to tell the person that's providing the code to go back to the drawing board and break it up?  Isn't that our job as committers?  And as the rest of the committers, isn't it our job to remind the offending committer to spend more time with the person who's providing the code so that we don't have to dig thru this much mess?
>> It's not always easy for any of us, but I just don't see this from other people, so I wanted to remind Hans.  If all the other committers think this is ok (which obviously they didn't since Adam brought it up), then I'll happily back off.  Since that's not the case - and this obviously doesn't follow the best practices of the project - please let me know how I should encourage Hans to do what is clearly in our best practices. What should be done is that this mess of a commit should be reverted and put back in in pieces - you have to start somewhere and sometimes it's just not good enough when you have an entire community to just say you're trying hard.  My two cents.
>> Cheers,
>> Ruppert
>> --
>> Tim Ruppert
>> HotWax Media
>> http://www.hotwaxmedia.com
>> o:801.649.6594
>> f:801.649.6595
>> On Jan 25, 2010, at 8:47 PM, David E Jones wrote:
>>> This seems to be a little bit condescending. Trying to pick another fight?
>>>
>>> I'm just glad to have ring-side seats... ;)
>>>
>>> -David
>>>
>>>
>>> On Jan 25, 2010, at 9:39 PM, Tim Ruppert wrote:
>>>
>>>> As the committer, you can just not put them into OFBiz until they are of better quality Hans - that is your obligation and something that the rest of us abide to.  Please do better in the future - we know you're trying - but just because you've received it doesn't mean you have to commit it - you can just wait until it's in good enough shape to commit it ...
>>>>
>>>> Cheers,
>>>> Ruppert
>>>> --
>>>> Tim Ruppert
>>>> HotWax Media
>>>> http://www.hotwaxmedia.com
>>>>
>>>> o:801.649.6594
>>>> f:801.649.6595
>>>>
>>>> On Jan 25, 2010, at 4:28 PM, Hans Bakker wrote:
>>>>
>>>>> Hi Adam,
>>>>>
>>>>> comment accepted, i am trying to get my people to provide stuff in
>>>>> smaller pieces. Not always easy but we try to do better.
>>>>>
>>>>> Regards,
>>>>> hans
>>>>>
>>>>> On Mon, 2010-01-25 at 12:28 -0600, Adam Heath wrote:
>>>>>> [hidden email] wrote:
>>>>>>> Author: hansbak
>>>>>>> Date: Mon Jan 25 06:26:23 2010
>>>>>>> New Revision: 902716
>>>>>>>
>>>>>>> URL: http://svn.apache.org/viewvc?rev=902716&view=rev
>>>>>>> Log:
>>>>>>>
>>>>>>> function leave feedback and screen - function auto relist item and screen
>>>>>>> - Get  feedback data from ebay site and save into ofbiz.
>>>>>>> - Add rate pictures into framework/images/webapp/images
>>>>>>> - screen for show awaiting feedback and recent feedback from buyer
>>>>>>> - help screens are provided
>>>>>> this was a bad commit, too many things done at once.  Please try to be
>>>>>> considerate of long-term maintainability, having small commits makes
>>>>>> it easier to verify and debug.
>>>>> --
>>>>> Antwebsystems.com: Quality OFBiz services for competitive rates
>>>>>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Adrian Crum
Jacopo,

No one is trying to make war. The details have been pointed out in
previous threads and in Jira issues.

-Adrian


Jacopo Cappellato wrote:

> I understand the concerns and remarks but please, let's try to stay focused on specific details (that needs to be fixed): only in this way the conversation will be useful and we will help each other to get better code.
> If instead we make general assumptions on other's work (like defining the effort of a committer "a mess" or the effort of a "beginner programmer") we will just end up with people being offended and defensive.
>
> "make code not war" :-)
>
> Jacopo
>
> On Jan 26, 2010, at 4:44 PM, Adrian Crum wrote:
>
>> Tim,
>>
>> I didn't take your remark as condescending - in fact I agree with you. It isn't okay for any committer to simply pass the buck (or blame).
>>
>> If Hans seems to be a frequent target for commit remarks it is because he earned it. There have been too many times when he has checked in code that was written by beginner programmers without reviewing it first. Or he has changed code without understanding it first. Those behaviors have had a detrimental effect on the project.
>>
>> Please don't stop pushing for code quality and responsible commits. You have my support.
>>
>> -Adrian
>>
>>
>> Tim Ruppert wrote:
>>> Not trying to - but sorry for the tone if it came across that way Hans.  I'm simply asking why it gets past the committer's desk as "I'm trying to get my people to provide smaller pieces" is an ok way of committing code.  Isn't that the responsibility of the committer to tell the person that's providing the code to go back to the drawing board and break it up?  Isn't that our job as committers?  And as the rest of the committers, isn't it our job to remind the offending committer to spend more time with the person who's providing the code so that we don't have to dig thru this much mess?
>>> It's not always easy for any of us, but I just don't see this from other people, so I wanted to remind Hans.  If all the other committers think this is ok (which obviously they didn't since Adam brought it up), then I'll happily back off.  Since that's not the case - and this obviously doesn't follow the best practices of the project - please let me know how I should encourage Hans to do what is clearly in our best practices. What should be done is that this mess of a commit should be reverted and put back in in pieces - you have to start somewhere and sometimes it's just not good enough when you have an entire community to just say you're trying hard.  My two cents.
>>> Cheers,
>>> Ruppert
>>> --
>>> Tim Ruppert
>>> HotWax Media
>>> http://www.hotwaxmedia.com
>>> o:801.649.6594
>>> f:801.649.6595
>>> On Jan 25, 2010, at 8:47 PM, David E Jones wrote:
>>>> This seems to be a little bit condescending. Trying to pick another fight?
>>>>
>>>> I'm just glad to have ring-side seats... ;)
>>>>
>>>> -David
>>>>
>>>>
>>>> On Jan 25, 2010, at 9:39 PM, Tim Ruppert wrote:
>>>>
>>>>> As the committer, you can just not put them into OFBiz until they are of better quality Hans - that is your obligation and something that the rest of us abide to.  Please do better in the future - we know you're trying - but just because you've received it doesn't mean you have to commit it - you can just wait until it's in good enough shape to commit it ...
>>>>>
>>>>> Cheers,
>>>>> Ruppert
>>>>> --
>>>>> Tim Ruppert
>>>>> HotWax Media
>>>>> http://www.hotwaxmedia.com
>>>>>
>>>>> o:801.649.6594
>>>>> f:801.649.6595
>>>>>
>>>>> On Jan 25, 2010, at 4:28 PM, Hans Bakker wrote:
>>>>>
>>>>>> Hi Adam,
>>>>>>
>>>>>> comment accepted, i am trying to get my people to provide stuff in
>>>>>> smaller pieces. Not always easy but we try to do better.
>>>>>>
>>>>>> Regards,
>>>>>> hans
>>>>>>
>>>>>> On Mon, 2010-01-25 at 12:28 -0600, Adam Heath wrote:
>>>>>>> [hidden email] wrote:
>>>>>>>> Author: hansbak
>>>>>>>> Date: Mon Jan 25 06:26:23 2010
>>>>>>>> New Revision: 902716
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/viewvc?rev=902716&view=rev
>>>>>>>> Log:
>>>>>>>>
>>>>>>>> function leave feedback and screen - function auto relist item and screen
>>>>>>>> - Get  feedback data from ebay site and save into ofbiz.
>>>>>>>> - Add rate pictures into framework/images/webapp/images
>>>>>>>> - screen for show awaiting feedback and recent feedback from buyer
>>>>>>>> - help screens are provided
>>>>>>> this was a bad commit, too many things done at once.  Please try to be
>>>>>>> considerate of long-term maintainability, having small commits makes
>>>>>>> it easier to verify and debug.
>>>>>> --
>>>>>> Antwebsystems.com: Quality OFBiz services for competitive rates
>>>>>>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Jacopo Cappellato-4

On Jan 26, 2010, at 5:21 PM, Adrian Crum wrote:

> Jacopo,
>
> No one is trying to make war. The details have been pointed out in previous threads and in Jira issues.
>

Yes, I am aware of this; and this is the right way to go, this with a lot of patience and perseverance
And just to be super clear: I was not accusing you (or anyone else) of trying to fight; I was just saying that general statements like the ones I have noticed could cause people being defensive, trying to demonstrate they are right instead of trying to understand how to make better code.

Jacopo

> -Adrian
>
>
> Jacopo Cappellato wrote:
>> I understand the concerns and remarks but please, let's try to stay focused on specific details (that needs to be fixed): only in this way the conversation will be useful and we will help each other to get better code.
>> If instead we make general assumptions on other's work (like defining the effort of a committer "a mess" or the effort of a "beginner programmer") we will just end up with people being offended and defensive.
>> "make code not war" :-)
>> Jacopo
>> On Jan 26, 2010, at 4:44 PM, Adrian Crum wrote:
>>> Tim,
>>>
>>> I didn't take your remark as condescending - in fact I agree with you. It isn't okay for any committer to simply pass the buck (or blame).
>>>
>>> If Hans seems to be a frequent target for commit remarks it is because he earned it. There have been too many times when he has checked in code that was written by beginner programmers without reviewing it first. Or he has changed code without understanding it first. Those behaviors have had a detrimental effect on the project.
>>>
>>> Please don't stop pushing for code quality and responsible commits. You have my support.
>>>
>>> -Adrian
>>>
>>>
>>> Tim Ruppert wrote:
>>>> Not trying to - but sorry for the tone if it came across that way Hans.  I'm simply asking why it gets past the committer's desk as "I'm trying to get my people to provide smaller pieces" is an ok way of committing code.  Isn't that the responsibility of the committer to tell the person that's providing the code to go back to the drawing board and break it up?  Isn't that our job as committers?  And as the rest of the committers, isn't it our job to remind the offending committer to spend more time with the person who's providing the code so that we don't have to dig thru this much mess?
>>>> It's not always easy for any of us, but I just don't see this from other people, so I wanted to remind Hans.  If all the other committers think this is ok (which obviously they didn't since Adam brought it up), then I'll happily back off.  Since that's not the case - and this obviously doesn't follow the best practices of the project - please let me know how I should encourage Hans to do what is clearly in our best practices. What should be done is that this mess of a commit should be reverted and put back in in pieces - you have to start somewhere and sometimes it's just not good enough when you have an entire community to just say you're trying hard.  My two cents.
>>>> Cheers,
>>>> Ruppert
>>>> --
>>>> Tim Ruppert
>>>> HotWax Media
>>>> http://www.hotwaxmedia.com
>>>> o:801.649.6594
>>>> f:801.649.6595
>>>> On Jan 25, 2010, at 8:47 PM, David E Jones wrote:
>>>>> This seems to be a little bit condescending. Trying to pick another fight?
>>>>>
>>>>> I'm just glad to have ring-side seats... ;)
>>>>>
>>>>> -David
>>>>>
>>>>>
>>>>> On Jan 25, 2010, at 9:39 PM, Tim Ruppert wrote:
>>>>>
>>>>>> As the committer, you can just not put them into OFBiz until they are of better quality Hans - that is your obligation and something that the rest of us abide to.  Please do better in the future - we know you're trying - but just because you've received it doesn't mean you have to commit it - you can just wait until it's in good enough shape to commit it ...
>>>>>>
>>>>>> Cheers,
>>>>>> Ruppert
>>>>>> --
>>>>>> Tim Ruppert
>>>>>> HotWax Media
>>>>>> http://www.hotwaxmedia.com
>>>>>>
>>>>>> o:801.649.6594
>>>>>> f:801.649.6595
>>>>>>
>>>>>> On Jan 25, 2010, at 4:28 PM, Hans Bakker wrote:
>>>>>>
>>>>>>> Hi Adam,
>>>>>>>
>>>>>>> comment accepted, i am trying to get my people to provide stuff in
>>>>>>> smaller pieces. Not always easy but we try to do better.
>>>>>>>
>>>>>>> Regards,
>>>>>>> hans
>>>>>>>
>>>>>>> On Mon, 2010-01-25 at 12:28 -0600, Adam Heath wrote:
>>>>>>>> [hidden email] wrote:
>>>>>>>>> Author: hansbak
>>>>>>>>> Date: Mon Jan 25 06:26:23 2010
>>>>>>>>> New Revision: 902716
>>>>>>>>>
>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=902716&view=rev
>>>>>>>>> Log:
>>>>>>>>>
>>>>>>>>> function leave feedback and screen - function auto relist item and screen
>>>>>>>>> - Get  feedback data from ebay site and save into ofbiz.
>>>>>>>>> - Add rate pictures into framework/images/webapp/images
>>>>>>>>> - screen for show awaiting feedback and recent feedback from buyer
>>>>>>>>> - help screens are provided
>>>>>>>> this was a bad commit, too many things done at once.  Please try to be
>>>>>>>> considerate of long-term maintainability, having small commits makes
>>>>>>>> it easier to verify and debug.
>>>>>>> --
>>>>>>> Antwebsystems.com: Quality OFBiz services for competitive rates
>>>>>>>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

David E. Jones-2
In reply to this post by Jacopo Cappellato-4

Thanks Jacopo this makes a good distinction.

We need peer review, and LOTS of it. We can't "wait" for code to improve because code doesn't just automatically improve over time, someone has to improve it and specific feedback on code itself will help with that.

I don't see any reason to talk about the people or guesses about the behavior that caused code problems (which is silly, how do you know?).

I'd argue that much of the feedback to Hans has not been specific or focused on the code, but rather either personal things or guesses at behavior behind the code or general trends that are of limited usefulness when it comes to improving the code. What's the point of any of that?

-David


On Jan 26, 2010, at 10:10 AM, Jacopo Cappellato wrote:

> I understand the concerns and remarks but please, let's try to stay focused on specific details (that needs to be fixed): only in this way the conversation will be useful and we will help each other to get better code.
> If instead we make general assumptions on other's work (like defining the effort of a committer "a mess" or the effort of a "beginner programmer") we will just end up with people being offended and defensive.
>
> "make code not war" :-)
>
> Jacopo
>
> On Jan 26, 2010, at 4:44 PM, Adrian Crum wrote:
>
>> Tim,
>>
>> I didn't take your remark as condescending - in fact I agree with you. It isn't okay for any committer to simply pass the buck (or blame).
>>
>> If Hans seems to be a frequent target for commit remarks it is because he earned it. There have been too many times when he has checked in code that was written by beginner programmers without reviewing it first. Or he has changed code without understanding it first. Those behaviors have had a detrimental effect on the project.
>>
>> Please don't stop pushing for code quality and responsible commits. You have my support.
>>
>> -Adrian
>>
>>
>> Tim Ruppert wrote:
>>> Not trying to - but sorry for the tone if it came across that way Hans.  I'm simply asking why it gets past the committer's desk as "I'm trying to get my people to provide smaller pieces" is an ok way of committing code.  Isn't that the responsibility of the committer to tell the person that's providing the code to go back to the drawing board and break it up?  Isn't that our job as committers?  And as the rest of the committers, isn't it our job to remind the offending committer to spend more time with the person who's providing the code so that we don't have to dig thru this much mess?
>>> It's not always easy for any of us, but I just don't see this from other people, so I wanted to remind Hans.  If all the other committers think this is ok (which obviously they didn't since Adam brought it up), then I'll happily back off.  Since that's not the case - and this obviously doesn't follow the best practices of the project - please let me know how I should encourage Hans to do what is clearly in our best practices. What should be done is that this mess of a commit should be reverted and put back in in pieces - you have to start somewhere and sometimes it's just not good enough when you have an entire community to just say you're trying hard.  My two cents.
>>> Cheers,
>>> Ruppert
>>> --
>>> Tim Ruppert
>>> HotWax Media
>>> http://www.hotwaxmedia.com
>>> o:801.649.6594
>>> f:801.649.6595
>>> On Jan 25, 2010, at 8:47 PM, David E Jones wrote:
>>>> This seems to be a little bit condescending. Trying to pick another fight?
>>>>
>>>> I'm just glad to have ring-side seats... ;)
>>>>
>>>> -David
>>>>
>>>>
>>>> On Jan 25, 2010, at 9:39 PM, Tim Ruppert wrote:
>>>>
>>>>> As the committer, you can just not put them into OFBiz until they are of better quality Hans - that is your obligation and something that the rest of us abide to.  Please do better in the future - we know you're trying - but just because you've received it doesn't mean you have to commit it - you can just wait until it's in good enough shape to commit it ...
>>>>>
>>>>> Cheers,
>>>>> Ruppert
>>>>> --
>>>>> Tim Ruppert
>>>>> HotWax Media
>>>>> http://www.hotwaxmedia.com
>>>>>
>>>>> o:801.649.6594
>>>>> f:801.649.6595
>>>>>
>>>>> On Jan 25, 2010, at 4:28 PM, Hans Bakker wrote:
>>>>>
>>>>>> Hi Adam,
>>>>>>
>>>>>> comment accepted, i am trying to get my people to provide stuff in
>>>>>> smaller pieces. Not always easy but we try to do better.
>>>>>>
>>>>>> Regards,
>>>>>> hans
>>>>>>
>>>>>> On Mon, 2010-01-25 at 12:28 -0600, Adam Heath wrote:
>>>>>>> [hidden email] wrote:
>>>>>>>> Author: hansbak
>>>>>>>> Date: Mon Jan 25 06:26:23 2010
>>>>>>>> New Revision: 902716
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/viewvc?rev=902716&view=rev
>>>>>>>> Log:
>>>>>>>>
>>>>>>>> function leave feedback and screen - function auto relist item and screen
>>>>>>>> - Get  feedback data from ebay site and save into ofbiz.
>>>>>>>> - Add rate pictures into framework/images/webapp/images
>>>>>>>> - screen for show awaiting feedback and recent feedback from buyer
>>>>>>>> - help screens are provided
>>>>>>> this was a bad commit, too many things done at once.  Please try to be
>>>>>>> considerate of long-term maintainability, having small commits makes
>>>>>>> it easier to verify and debug.
>>>>>> --
>>>>>> Antwebsystems.com: Quality OFBiz services for competitive rates
>>>>>>
>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Adrian Crum
For those of you who didn't bother to read the entire thread, the
comments in it were specific:

1. Hans made a large commit
2. Adam criticized the commit, asked Hans to break it up into smaller pieces
3. Hans blamed one of his workers
4. Tim commented that Hans has an obligation to make good commits
5. David accused Tim of picking a fight
6. Tim responded to David's accusation
7. I supported Tim, saying Hans' reputation has earned him additional
scrutiny

I understand David and Jacopo are trying to be diplomatic, but in this
particular case it is counter-productive. Sometimes you just have to
call a spade a spade.

I would go into more detail about why I said what I did about Hans, but
right now I'm too busy fixing the code he broke.

-Adrian

David E Jones wrote:

> Thanks Jacopo this makes a good distinction.
>
> We need peer review, and LOTS of it. We can't "wait" for code to improve because code doesn't just automatically improve over time, someone has to improve it and specific feedback on code itself will help with that.
>
> I don't see any reason to talk about the people or guesses about the behavior that caused code problems (which is silly, how do you know?).
>
> I'd argue that much of the feedback to Hans has not been specific or focused on the code, but rather either personal things or guesses at behavior behind the code or general trends that are of limited usefulness when it comes to improving the code. What's the point of any of that?
>
> -David
>
>
> On Jan 26, 2010, at 10:10 AM, Jacopo Cappellato wrote:
>
>> I understand the concerns and remarks but please, let's try to stay focused on specific details (that needs to be fixed): only in this way the conversation will be useful and we will help each other to get better code.
>> If instead we make general assumptions on other's work (like defining the effort of a committer "a mess" or the effort of a "beginner programmer") we will just end up with people being offended and defensive.
>>
>> "make code not war" :-)
>>
>> Jacopo
>>
>> On Jan 26, 2010, at 4:44 PM, Adrian Crum wrote:
>>
>>> Tim,
>>>
>>> I didn't take your remark as condescending - in fact I agree with you. It isn't okay for any committer to simply pass the buck (or blame).
>>>
>>> If Hans seems to be a frequent target for commit remarks it is because he earned it. There have been too many times when he has checked in code that was written by beginner programmers without reviewing it first. Or he has changed code without understanding it first. Those behaviors have had a detrimental effect on the project.
>>>
>>> Please don't stop pushing for code quality and responsible commits. You have my support.
>>>
>>> -Adrian
>>>
>>>
>>> Tim Ruppert wrote:
>>>> Not trying to - but sorry for the tone if it came across that way Hans.  I'm simply asking why it gets past the committer's desk as "I'm trying to get my people to provide smaller pieces" is an ok way of committing code.  Isn't that the responsibility of the committer to tell the person that's providing the code to go back to the drawing board and break it up?  Isn't that our job as committers?  And as the rest of the committers, isn't it our job to remind the offending committer to spend more time with the person who's providing the code so that we don't have to dig thru this much mess?
>>>> It's not always easy for any of us, but I just don't see this from other people, so I wanted to remind Hans.  If all the other committers think this is ok (which obviously they didn't since Adam brought it up), then I'll happily back off.  Since that's not the case - and this obviously doesn't follow the best practices of the project - please let me know how I should encourage Hans to do what is clearly in our best practices. What should be done is that this mess of a commit should be reverted and put back in in pieces - you have to start somewhere and sometimes it's just not good enough when you have an entire community to just say you're trying hard.  My two cents.
>>>> Cheers,
>>>> Ruppert
>>>> --
>>>> Tim Ruppert
>>>> HotWax Media
>>>> http://www.hotwaxmedia.com
>>>> o:801.649.6594
>>>> f:801.649.6595
>>>> On Jan 25, 2010, at 8:47 PM, David E Jones wrote:
>>>>> This seems to be a little bit condescending. Trying to pick another fight?
>>>>>
>>>>> I'm just glad to have ring-side seats... ;)
>>>>>
>>>>> -David
>>>>>
>>>>>
>>>>> On Jan 25, 2010, at 9:39 PM, Tim Ruppert wrote:
>>>>>
>>>>>> As the committer, you can just not put them into OFBiz until they are of better quality Hans - that is your obligation and something that the rest of us abide to.  Please do better in the future - we know you're trying - but just because you've received it doesn't mean you have to commit it - you can just wait until it's in good enough shape to commit it ...
>>>>>>
>>>>>> Cheers,
>>>>>> Ruppert
>>>>>> --
>>>>>> Tim Ruppert
>>>>>> HotWax Media
>>>>>> http://www.hotwaxmedia.com
>>>>>>
>>>>>> o:801.649.6594
>>>>>> f:801.649.6595
>>>>>>
>>>>>> On Jan 25, 2010, at 4:28 PM, Hans Bakker wrote:
>>>>>>
>>>>>>> Hi Adam,
>>>>>>>
>>>>>>> comment accepted, i am trying to get my people to provide stuff in
>>>>>>> smaller pieces. Not always easy but we try to do better.
>>>>>>>
>>>>>>> Regards,
>>>>>>> hans
>>>>>>>
>>>>>>> On Mon, 2010-01-25 at 12:28 -0600, Adam Heath wrote:
>>>>>>>> [hidden email] wrote:
>>>>>>>>> Author: hansbak
>>>>>>>>> Date: Mon Jan 25 06:26:23 2010
>>>>>>>>> New Revision: 902716
>>>>>>>>>
>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=902716&view=rev
>>>>>>>>> Log:
>>>>>>>>>
>>>>>>>>> function leave feedback and screen - function auto relist item and screen
>>>>>>>>> - Get  feedback data from ebay site and save into ofbiz.
>>>>>>>>> - Add rate pictures into framework/images/webapp/images
>>>>>>>>> - screen for show awaiting feedback and recent feedback from buyer
>>>>>>>>> - help screens are provided
>>>>>>>> this was a bad commit, too many things done at once.  Please try to be
>>>>>>>> considerate of long-term maintainability, having small commits makes
>>>>>>>> it easier to verify and debug.
>>>>>>> --
>>>>>>> Antwebsystems.com: Quality OFBiz services for competitive rates
>>>>>>>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

David E. Jones-2

Adrian,

You have a gift for accuracy, generosity, and refreshingly bias-free analysis.

-David


On Jan 26, 2010, at 11:42 AM, Adrian Crum wrote:

> For those of you who didn't bother to read the entire thread, the comments in it were specific:
>
> 1. Hans made a large commit
> 2. Adam criticized the commit, asked Hans to break it up into smaller pieces
> 3. Hans blamed one of his workers
> 4. Tim commented that Hans has an obligation to make good commits
> 5. David accused Tim of picking a fight
> 6. Tim responded to David's accusation
> 7. I supported Tim, saying Hans' reputation has earned him additional scrutiny
>
> I understand David and Jacopo are trying to be diplomatic, but in this particular case it is counter-productive. Sometimes you just have to call a spade a spade.
>
> I would go into more detail about why I said what I did about Hans, but right now I'm too busy fixing the code he broke.
>
> -Adrian
>
> David E Jones wrote:
>> Thanks Jacopo this makes a good distinction.
>> We need peer review, and LOTS of it. We can't "wait" for code to improve because code doesn't just automatically improve over time, someone has to improve it and specific feedback on code itself will help with that.
>> I don't see any reason to talk about the people or guesses about the behavior that caused code problems (which is silly, how do you know?).
>> I'd argue that much of the feedback to Hans has not been specific or focused on the code, but rather either personal things or guesses at behavior behind the code or general trends that are of limited usefulness when it comes to improving the code. What's the point of any of that?
>> -David
>> On Jan 26, 2010, at 10:10 AM, Jacopo Cappellato wrote:
>>> I understand the concerns and remarks but please, let's try to stay focused on specific details (that needs to be fixed): only in this way the conversation will be useful and we will help each other to get better code.
>>> If instead we make general assumptions on other's work (like defining the effort of a committer "a mess" or the effort of a "beginner programmer") we will just end up with people being offended and defensive.
>>>
>>> "make code not war" :-)
>>>
>>> Jacopo
>>>
>>> On Jan 26, 2010, at 4:44 PM, Adrian Crum wrote:
>>>
>>>> Tim,
>>>>
>>>> I didn't take your remark as condescending - in fact I agree with you. It isn't okay for any committer to simply pass the buck (or blame).
>>>>
>>>> If Hans seems to be a frequent target for commit remarks it is because he earned it. There have been too many times when he has checked in code that was written by beginner programmers without reviewing it first. Or he has changed code without understanding it first. Those behaviors have had a detrimental effect on the project.
>>>>
>>>> Please don't stop pushing for code quality and responsible commits. You have my support.
>>>>
>>>> -Adrian
>>>>
>>>>
>>>> Tim Ruppert wrote:
>>>>> Not trying to - but sorry for the tone if it came across that way Hans.  I'm simply asking why it gets past the committer's desk as "I'm trying to get my people to provide smaller pieces" is an ok way of committing code.  Isn't that the responsibility of the committer to tell the person that's providing the code to go back to the drawing board and break it up?  Isn't that our job as committers?  And as the rest of the committers, isn't it our job to remind the offending committer to spend more time with the person who's providing the code so that we don't have to dig thru this much mess?
>>>>> It's not always easy for any of us, but I just don't see this from other people, so I wanted to remind Hans.  If all the other committers think this is ok (which obviously they didn't since Adam brought it up), then I'll happily back off.  Since that's not the case - and this obviously doesn't follow the best practices of the project - please let me know how I should encourage Hans to do what is clearly in our best practices. What should be done is that this mess of a commit should be reverted and put back in in pieces - you have to start somewhere and sometimes it's just not good enough when you have an entire community to just say you're trying hard.  My two cents.
>>>>> Cheers,
>>>>> Ruppert
>>>>> --
>>>>> Tim Ruppert
>>>>> HotWax Media
>>>>> http://www.hotwaxmedia.com
>>>>> o:801.649.6594
>>>>> f:801.649.6595
>>>>> On Jan 25, 2010, at 8:47 PM, David E Jones wrote:
>>>>>> This seems to be a little bit condescending. Trying to pick another fight?
>>>>>>
>>>>>> I'm just glad to have ring-side seats... ;)
>>>>>>
>>>>>> -David
>>>>>>
>>>>>>
>>>>>> On Jan 25, 2010, at 9:39 PM, Tim Ruppert wrote:
>>>>>>
>>>>>>> As the committer, you can just not put them into OFBiz until they are of better quality Hans - that is your obligation and something that the rest of us abide to.  Please do better in the future - we know you're trying - but just because you've received it doesn't mean you have to commit it - you can just wait until it's in good enough shape to commit it ...
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Ruppert
>>>>>>> --
>>>>>>> Tim Ruppert
>>>>>>> HotWax Media
>>>>>>> http://www.hotwaxmedia.com
>>>>>>>
>>>>>>> o:801.649.6594
>>>>>>> f:801.649.6595
>>>>>>>
>>>>>>> On Jan 25, 2010, at 4:28 PM, Hans Bakker wrote:
>>>>>>>
>>>>>>>> Hi Adam,
>>>>>>>>
>>>>>>>> comment accepted, i am trying to get my people to provide stuff in
>>>>>>>> smaller pieces. Not always easy but we try to do better.
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> hans
>>>>>>>>
>>>>>>>> On Mon, 2010-01-25 at 12:28 -0600, Adam Heath wrote:
>>>>>>>>> [hidden email] wrote:
>>>>>>>>>> Author: hansbak
>>>>>>>>>> Date: Mon Jan 25 06:26:23 2010
>>>>>>>>>> New Revision: 902716
>>>>>>>>>>
>>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=902716&view=rev
>>>>>>>>>> Log:
>>>>>>>>>>
>>>>>>>>>> function leave feedback and screen - function auto relist item and screen
>>>>>>>>>> - Get  feedback data from ebay site and save into ofbiz.
>>>>>>>>>> - Add rate pictures into framework/images/webapp/images
>>>>>>>>>> - screen for show awaiting feedback and recent feedback from buyer
>>>>>>>>>> - help screens are provided
>>>>>>>>> this was a bad commit, too many things done at once.  Please try to be
>>>>>>>>> considerate of long-term maintainability, having small commits makes
>>>>>>>>> it easier to verify and debug.
>>>>>>>> --
>>>>>>>> Antwebsystems.com: Quality OFBiz services for competitive rates
>>>>>>>>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Adam Heath-2
In reply to this post by Tim Ruppert
Tim Ruppert wrote:
> Not trying to - but sorry for the tone if it came across that way Hans.  I'm simply asking why it gets past the committer's desk as "I'm trying to get my people to provide smaller pieces" is an ok way of committing code.  Isn't that the responsibility of the committer to tell the person that's providing the code to go back to the drawing board and break it up?  Isn't that our job as committers?  And as the rest of the committers, isn't it our job to remind the offending committer to spend more time with the person who's providing the code so that we don't have to dig thru this much mess?
>
> It's not always easy for any of us, but I just don't see this from other people, so I wanted to remind Hans.  If all the other committers think this is ok (which obviously they didn't since Adam brought it up), then I'll happily back off.  Since that's not the case - and this obviously doesn't follow the best practices of the project - please let me know how I should encourage Hans to do what is clearly in our best practices.
>
> What should be done is that this mess of a commit should be reverted and put back in in pieces - you have to start somewhere and sometimes it's just not good enough when you have an entire community to just say you're trying hard.  My two cents.

Reverting the commit is wrong.  Just try to be more careful in the future.
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Adam Heath-2
In reply to this post by Adrian Crum
Adrian Crum wrote:
> Jacopo,
>
> No one is trying to make war. The details have been pointed out in
> previous threads and in Jira issues.

make: *** No rule to make target `war'.  Stop.
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Adam Heath-2
In reply to this post by David E. Jones-2
David E Jones wrote:
> Thanks Jacopo this makes a good distinction.
>
> We need peer review, and LOTS of it. We can't "wait" for code to improve because code doesn't just automatically improve over time, someone has to improve it and specific feedback on code itself will help with that.
>
> I don't see any reason to talk about the people or guesses about the behavior that caused code problems (which is silly, how do you know?).
>
> I'd argue that much of the feedback to Hans has not been specific or focused on the code, but rather either personal things or guesses at behavior behind the code or general trends that are of limited usefulness when it comes to improving the code. What's the point of any of that?

I haven't done any of this last paragraph.  In fact, when I generally
reply to a commit mail, I tend to have blinders on, as to who actually
did it, what their background is, where they are located, what
timezone, etc.

If I suddenly start remembing who did what, then it might make me
start writing emails in a more personal matter, which then could be
misinterpeted, since we all have different backgrounds.  It's much
better to look at *just the code*; it's much harder to argue against
that for argumentative sake.
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Adam Heath-2
In reply to this post by Adrian Crum
Adrian Crum wrote:

> For those of you who didn't bother to read the entire thread, the
> comments in it were specific:
>
> 1. Hans made a large commit
> 2. Adam criticized the commit, asked Hans to break it up into smaller
> pieces
> 3. Hans blamed one of his workers
> 4. Tim commented that Hans has an obligation to make good commits
> 5. David accused Tim of picking a fight
> 6. Tim responded to David's accusation
> 7. I supported Tim, saying Hans' reputation has earned him additional
> scrutiny

Except that in step 4, Tim mentioned Hans; if he had removed that
single word, I would have sided(I hate that term in this situation)
with Tim.

Yes, is a very small change.  But if you re-read my mail, I didn't
mention any particular person.  I just used generic pronouns.  I do
that, so my explanations can be applied to *anyone*, as we *all* must
follow these rules.

When a lone person is singled out, it is taken to mean that the rules
or discussion only apply to the target, instead of all.
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ specialpurpose/ebay/servicedef/ specialpurpose/ebay/src/org/...

Adrian Crum
Adam Heath wrote:

> Adrian Crum wrote:
>> For those of you who didn't bother to read the entire thread, the
>> comments in it were specific:
>>
>> 1. Hans made a large commit
>> 2. Adam criticized the commit, asked Hans to break it up into smaller
>> pieces
>> 3. Hans blamed one of his workers
>> 4. Tim commented that Hans has an obligation to make good commits
>> 5. David accused Tim of picking a fight
>> 6. Tim responded to David's accusation
>> 7. I supported Tim, saying Hans' reputation has earned him additional
>> scrutiny
>
> Except that in step 4, Tim mentioned Hans; if he had removed that
> single word, I would have sided(I hate that term in this situation)
> with Tim.
>
> Yes, is a very small change.  But if you re-read my mail, I didn't
> mention any particular person.  I just used generic pronouns.  I do
> that, so my explanations can be applied to *anyone*, as we *all* must
> follow these rules.
>
> When a lone person is singled out, it is taken to mean that the rules
> or discussion only apply to the target, instead of all.

Very good advice. I will strive to be less personal in the future.
123