[haiku-commits] Re: haiku: hrev47567 - src/kits/interface
- From: John Scipione <jscipione@xxxxxxxxx>
- To: "haiku-commits@xxxxxxxxxxxxx" <haiku-commits@xxxxxxxxxxxxx>
- Date: Mon, 28 Jul 2014 14:54:34 -0400
On Mon, Jul 28, 2014 at 6:27 AM, Axel Dörfler <axeld@xxxxxxxxxxxxxxxx> wrote:
>> On July 25, 2014 at 1:53 AM John Scipione <jscipione@xxxxxxxxx> wrote:
>> Axel, calm down and take a deep breath.
>>
>> I didn't figure out the problem at first but after some more research
>> I learned what was going on and corrected the array size to be 5
>> rather than 4 in hrev47568. [1]
>
> I noticed that later, but it's really annoying that you:
> 1) commit before doing any research, but only start researching after the
> commit
> 2) are obviously not able to listen at all. What you wrote there in your 2nd
> commit message is pretty much exactly what I told you in IRC. Even including
> the
> byte sizes, and the commit that broke it.
I apologize again that I misunderstood what you wrote on IRC, since I
did not keep a log of the conversation I can only assume that you
pointed out the problem clearly and that I must have misinterpreted
what you said.
I didn't think I _had_ to do research as I was going off your guidance
which I assumed to be correct. I was able to ascertain the true source
of the problem only after doing research (and writing code) in
response to Stefano Ceccherini's comment (thanks Stefano).
> If you are so weak at remembering/reading things, you should adapt your
> workflow
> accordingly.
> Since it's obviously futile, I'll stop communicating issues to you this way,
> but
> open Trac tickets instead where the information is steadily available for you
> to
> pick up as often as needed.
Thank you for taking the time to report the problem to me, I try to
fix any problems that I am made aware of as soon as possible. I want
to make it clear that I really do appreciate that you took the time to
report the problem to me and that you have demonstrated great the
patience even though you are also a bit upset.
A bug report is a good way to disseminate information, but takes a bit
more work on your part. I realize that you have limited time so
reporting a problem on IRC can be faster and easier. I'm not going to
tell you that you must write a bug report for each problem that you
wish to report to me, but, if that is your decision I will not try to
stop you.
I've had others report problems to me on IRC and if I think I've got a
handle on the cause I'll fix the problem without any further ceremony
but if I don't have time or I have an incomplete understanding of the
problem I'll ask them to file a bug report. In this case I thought
that I had a handle on the problem but didn't.
> At least you learned something about FBC in the process which was the only
> reason not to fix it myself. Hopefully it'll last a bit.
No, I haven't really, it's still a black box to me. I have yet to find
a definitive resource on the problem. I looked in "Effective C++"
Third Edition by Scott Meyer's and "The C++ Programming Language"
Forth Edition by Bjarne Stroustrup and neither mention the fragile
base class (aka fragile inheritance) problem nor detail what modifies
the size and vtable of a class and how.
My understanding thus far is that the problem occurs when changing the
order of or adding or removing non-inherited virtual methods or adding
or removing private member variables from the class. However this is
all derived from conjecture, I would prefer to learn from a definitive
reference on the subject, but I haven't found one yet.
For instance I added 2 bools (1 byte each) while the class size went
from 356 bytes to 360 bytes, increasing by 4 bytes. This suggests that
there is some padding going on, but this isn't clear, is this compiler
dependent or something that can be relied on? If I were to add another
bool would the class size increase again, or would it stay 360 bytes
deducting from the padding? Perhaps it depends on where I add the bool
as it might pad differently?
I understand how struct padding works on x86 so I don't need you to
lecture me on the details, and I assume that class padding works
similarly, but, that is an assumption, one that I'd like to verify but
don't have a good resource to do so. I don't want you to explain it to
me here, but if you could point me to a good reference I'd appreciate
it.
>> > Is it really that hard to even read? Maybe rereading the IRC log before
>> > breaking everything just a bit differently would have made sense?
I think this is a bit of an exaggeration as I didn't break anything
that wasn't already broken. I made a mistake, then upon learning that
I made the mistake researched the problem and corrected it promptly.
What do you want me to do, never make a mistake? Well, I can assure
you that I will make mistakes again in the future, and I bet you will
to. I've corrected the mistake, I've apologized, I am moving on, I
hope that you can do the same.
[snip]
>> >> dc4ae0e: TextView: Fix regression for Home and End
>> >> ... while Shift is held down in a selection.
>> > Ouch, you didn't even manage to get this one right.
>> > THE BEHAVIOR IS BROKEN WITHOUT HOLDING DOWN SHIFT, TOO. And that was what I
>> > mentioned to you.
>> Again, calm down, I realize that there was a problem also when the
>> shift key was not held down and I solved the problem in both cases.
>
> Then please note that in the commit message. And also, please try to make your
> commit messages more concise.
I try to make the length of my commit message fit the severity of the
change involved. This commit involved a small change in terms of lines
of code, but, had a medium-sized impact which made it deserve a
medium-size commit message. I genuinely thought that I explained the
problem concisely and provided enough details to the reader to
understand the problem, the fix, and other possible fixes that were
explored.
> While I like the level of information you usually put in there, it would be
> great if you could try to do it with less words.
This might be a situation where "if I had more time I would have
written you a shorter letter". You have to believe me that I edited
the message down from a longer message to include only the most
salient details. I tried to put into as few words possible what
stippi's (very nice) diagrams conveyed and that wasn't easy.
> It's basically the same as with comments in the code: be as concise and
> precise
> as possible. Otherwise the information you want to deliver might be lost in
> the
> shear amount of text.
I agree, that is a good rule of thumb.
>> > At least nice that you fixed another regression, even though you seem to
>> > have borked it, too. The BeOS solution definitely makes sense. Please
>> > implement that one instead.
>> I actually implemented the BeOS solution as well but I was and am
>> reluctant to use it. There are pro's and con's on both sides and I am
>> not convinced that the BeOS solution is superior.
>
> I obviously did not understand how you implemented it. After having read
> Stephan's explanation (assuming that's how it works now), I think you did it
> right after all, so sorry for misjudging you not that nicely.
Is it really too much to ask that you load up Haiku and try the
feature before judging it? I don't want to single you out because I
feel as though I've been unfairly picked on before by others who judge
a change without even trying it first. With these UI type changes it's
something you have to grow into a bit to fully appreciate.
This was no simple fix, I did hours of research to figure out what the
proper fix should be, admitted that their were several possible ways
to fix the problem and provided an example of a promising alternative.
Maybe the BeOS way truly is better once you're used to it, maybe not,
my point is that it might be something you have to give a chance
before you can make a determination either way. If you just read the
commit message and react you don't give the feature a proper chance
potentially making the system a little bit worse for everyone. In this
case you reacted both without trying the feature as implemented and
without having a clear understanding of the alternative!
>> > Is it possible to forbid you to do any changes that change more than two
>> > characters? This is really more than just a little annoying.
>> I'm sorry that you are annoyed, all I can do is apologize and to try
>> and fix any issues that come up as soon as possible. Also, changing
>> the array size only involved changing 1 character and I managed to
>> still screw that up so I don't think that would completely solve the
>> problem.
>
> While that would make reviewing your commits substantially easier, I have to
> admit it's not a really good solution.
I'm afraid that either we need a more elaborate check-in process to
catch errors before they hit the main repo, or, we just all need to
calm down, take a deep breath and realize that each step gets us a bit
closer and that there will be some hiccups along the way. I see bad
commits come in followed by "build fix", bad style, regressions come
in and then get fixed all the time but somehow I have the fortitude to
continue so I vote for the latter, but YMMV.
Other related posts:
