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. > 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. > . > 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. > 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 when 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?" Kind regards, Jacopo
