On 05/21/2012 12:55 AM, Jacopo Cappellato wrote:
Hi Adam,
please see inline:
On May 20, 2012, at 6:02 PM, Adam Heath wrote:
You just made my job harder. I said I would do it, and I already had it done.
You actually wrote:
"I'll correct those..."
and you didn' say that you already had local changes and since I was already
working on this refactoring (that is a result of a research I started some
weeks ago in the Freemarker list), I had some time allocated in sunday and so I
decided to implement.
At time I sent that, I didn't have any changes. I did later that day.
I started a test run locally, against 5 changes, then 'went out'. Just
as I was about to leave my place of fun, I got an email notification on
my phone, and saw that you ignored my email.
Unlike some people(I'm not saying you), I do extensive testing of a
whole series of changes before committing; that takes time. You ignored
my email, and went full-steam ahead. That caused me additional work,
and even delayed me by a day, as I had to rework the set of changes I
had to deal with what you did.
I *tried* to communicate ahead of time. But that was ignored. You just
committed straight away, without giving me any chance to respond.
"I'll correct those" means I will do it soon, ie, in the future, not
that I already have it done. That would be "I have those corrected".
Now I have to fix my code to interact with your changes. And, due to the
following, almost undo your code.
You didn't read your diff, you left one instance of accessing the static
variable, instead of calling the static accessor method; granted, it's inside a
comment, but still, you should have double-checked your diff. I do.
This next one is my opinion: Putting 'ofbiz' into the method name is
superfluous. We already know it is an ofbiz class. No need to duplicate that
into the name of the method.
The method returns a static BeansWrapper. The name doesn't reflect that fact.
My local code has it named 'singletonBeansWrapper()'.
This is awful name, imo, but it is just a personal opinion as it is yours on
defaultOfbizWrapper (that I actually don't like much too, but it was the one
before my changes). As a side trivial note my preference would be:
defaultBeansWrapper.
I had it called that, because my series of changes also allowed for
other classes to maintain their own instance of a BeansWrapper; in fact,
ExtendedWrapper is one such case. I also called it singleton because it
more accurately reflects what it is for, then default.
I also didn't like the name singletonBeansWrapper; I probably would have
changed that before my final commit. I never got the chance, as you
invalidated it. I've decided to go with yours, even tho I disagree with
the name.
In summary, I'm rather annoyed.
I don't care about this; and I am actually very annoyed by the fact that the
applications are broken since a few days because of your commit, that you
clearly didn't even test and I have to report the issue to you. But before now
I didn't mention this because I understand that people do errors and the
direction of your work was good (even if you did errors that are causing
problems) and so I didn't stress this fact; and instead I am helping you to
have this issue fixed.
I gave plenty of warning ahead of time, and that it was not possible for
me to test everything thoroughly. I apologize for missing the example
that has been given(looking at any order).
Normally, I wouldn't be. But in this case, I said I would do this change,
and you went and did it anyways. I even gave a good reason for wanting to do
it, as I needed it anyways to implement the findByAnd fix.
Now, Adam, please read carefully what follows: I am asking you to immediately change attitude whehe
tin you interact with me (you can continue with others if you like, I don't care); the tone you are
using is annoying and unacceptable to me; be polite, do not pretend you can give me lessons (you
can't as the history of our interactions actually shows), but rather focus on specific details,
starting the email with "Hi Jacopo" could also help (and when applicable end it with
"Thank you").
Just to give you a clear example of something that you will have to change in
your style when you interact with me, instead of:
You didn't read your diff, you left one instance of accessing the static
variable, instead of calling the static accessor method; granted, it's inside a
comment, but still, you should have double-checked your diff. I do.
you could have written:
"There is a commented out code that has a direct reference to the variable, could
you please fix it?"
Why does everything have to be coated with sugar with you? You made a
mistake(committing something that I said I was working on). That error
is a completely separate issue with regard to anything else I might have
done(breaking ftl files). Except, that I didn't break ftl files,
freemarker has always been broken. I've actually fixed that now, so
that method calls in freemarker actually are more sane.
And, with regard to the sugar coating that you seem to so desperately
desire, you never apologized to me for stomping on my work.
The "Hi Adam" and signature are superfluous, imho. We know who the
email is from, that's part of the email header. The target of the mail
is either anonymous(most of the time), or the previous author of the
email being responded to. I don't repeat information that is quite
apparent, imho.
I don't generally ask others to do stuff. This is a volunteer project,
if I can do it myself, then I'll do it. You can't always expect a
timely response to requests. That's why I didn't ask you to continue
fixing the other BeansWrapper references, and that is why I didn't ask
you to fix that commented out version. I did them myself, because they
were trivial. But I said I would fix them publically, before I did it.
What kind of additionaly paperwork do I need to sign to make you
understand that?
If you would have responded to my mail from yesterday, saying you
already have a change done, that would have been fine. Instead, I just
see a commit from you, then I see your response. No indication that you
were also going to work on it, nor that you already had it local, or
anything of that nature.
Kind regards,
Jacopo
