[haiku-commits] Re: haiku: hrev53163 - src/apps/haiku3d/texture
- From: Dario Casalinuovo <b.vitruvio@xxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Sat, 25 May 2019 18:14:03 +0200
On Sat, May 25, 2019 at 5:16 PM waddlesplash <waddlesplash@xxxxxxxxx> wrote:
On Sat, May 25, 2019, 11:10 AM Dario Casalinuovo <b.vitruvio@xxxxxxxxx>
wrote:
Good evening,
- memset(&format, 0, sizeof(media_format));
+ format = media_format();
Out of curiosity: could you explain me why you added the Cleanup()
method, bypassing code reviews, and then not even use it?
I don't recall bypassing code reviews -- actually I didn't create the
Cleanup method, someone else did, and I just merged it. It seemed fine
then, anyway.
Pushing a commit without waiting for people feedback is bypassing code
reviews. But honestly, who cares, everyone can miss a hit. The pity is your
following behavior.
This is nice, actually this is the way I proposed at the time, but now I'm
a bit confused, did you change idea or what?
Is this what you were proposing? Well ... I can't recall that :/ It's
possible that you did in that thread, but I mostly stopped reading it after
the first dozen replies arguing about POD...
This is hilarious IMHO. The excuse for the next few years will be that you
didn't read the emails because [...]. But I want to ignore
the disengagement argument.
Let's just show the evidence, IRC snippet from #haiku-dev (lug means july) :
lug 11 16:05:22 <Barrett> if you want to reuse a media_format, just call
the constructor that will make the members zeroed
lug 11 16:05:34 <Barrett> finally inside the constructor there's no need to
call memset
lug 11 16:05:47 <Barrett> just make the members zero-ed
lug 11 16:06:11 <Barrett> the compiler will often just translate in the
same or very similar instructions that are used for memset
lug 11 16:06:23 <Barrett> but there are situations where things can be
screwed up.
lug 11 16:06:30 <Barrett> memset is C API.
lug 11 16:09:21 <Barrett> when we will switch to c++11 and when c++20 will
be there
lug 11 16:09:31 <Barrett> there will be more elegant way to make this work
lug 11 16:09:46 <Barrett> but for now that's the way to do the things
following the standard
lug 11 16:15:01 <waddlesplash> ok I guess it's probably fine then
Second snippet (at the time of the second ml discussion):
lug 25 15:51:15 <kallisti5> part of being a developer is code review
lug 25 15:51:18 <Barrett> as I explained you the other week
lug 25 15:51:20 <waddlesplash> ok, so, we can remove it in this case then
lug 25 15:51:24 <Barrett> format = media_format()
lug 25 15:51:25 <Barrett> that's all
lug 25 15:51:34 <Barrett> so it remains Unflatten
lug 25 15:51:35 <kallisti5> I'm not seeing anything really bad overall on
the ML
lug 25 15:51:45 <kallisti5> the comments have been minor
lug 25 15:51:46 <Barrett> there's no flatten, I'd move it into a c function
lug 25 15:51:55 <Barrett> but this is really not important
Third snippet, in this case I've been explaining it to kallisti5:
lug 25 16:03:12 <kallisti5> Barrett: let me ask this
lug 25 16:03:20 <kallisti5> "what's wrong with the change"
lug 25 16:03:26 <kallisti5> technically
lug 25 16:03:37 <Barrett> kallisti5, you see there's a memset in ctor right?
lug 25 16:04:05 <waddlesplash> ... in which case the code was wrong before
lug 25 16:04:15 <waddlesplash> or rather, superfluous
lug 25 16:04:21 <waddlesplash> and is equivalently superfluous now
lug 25 16:04:36 <kallisti5> Barrett: i do
lug 25 16:04:43 <Barrett> I never said the code was OK before
lug 25 16:04:48 <waddlesplash> but again, that's only half the code
lug 25 16:04:51 <Barrett> I am all for removing memset
lug 25 16:04:56 <waddlesplash> the code in the loop is correct
lug 25 16:05:01 <waddlesplash> and should be Clear indeed
lug 25 16:05:15 <Barrett> kallisti5, then instead of format.Clear() you do
format = media_format()
lug 25 16:05:26 <Barrett> so we removed completely the need of Clear()
lug 25 16:06:19 <Barrett> and only when this is needed
lug 25 16:06:21 <kallisti5> ok.. so two lines are wrong?
lug 25 16:06:33 <kallisti5> and the rest of the commit is ok?
lug 25 16:06:44 <Barrett> kallisti5, the previous commit that introduced
Clear is wrong too
lug 25 16:07:03 <Barrett> and I had a talk with waddlesplash explaining the
same things I explained to you, 2 weeks ago
lug 25 16:07:17 <Barrett> now, can I feel a bit joked to see this commit?
lug 25 16:07:18 <kallisti5> ok. Did you post these concerns to the ML?
What this log show is that you indeed read the whole conversation, by
comparing your claims to the discussion events. But okay, I can accept
someone may have missed it (two times?), no one is ubiquitous. But then I
tried to explain my point in a sincere and honest way.
I am so displeased that I used so much energy to deal with someone which
enjoy so much to troll me. What a waste.
--
Saluti,
Dario
Other related posts:
