[vlc-devel] [vlc-commits] JSON representation and Javascript unescaping

Rémi Denis-Courmont remi at remlab.net
Tue Sep 29 17:39:16 CEST 2020


Le tiistaina 29. syyskuuta 2020, 7.40.32 EEST Pierre Ynard via vlc-devel a 
écrit :
> I can't speak for others, but I would not have mentioned user agents
> since we already discussed it and you already expressed support for
> incrementally adding such features. I would not have complained about
> JSON since we already discussed it, and it was my preferred approach,
> and I read your reply about the subpar quality of the other JSON parser
> already in the codebase.

I was replying to Thomas, not you...

> > The reality is that I was in favor of getting the code reviewed at
> > first. Otherwise, I wouldn't even have sent it for review in the first
> > place. And then prerequisite inflation and filibuster disgusted me.
> > 
> > You can refuse to see those problems, or you can take a critical look
> > at everyone's attitude (as opposed to just mine) and actually try to
> > learn from that.
> 
> I understand that it's how you may have still been interpreting the
> situation. But what do you want people to do instead?
> Not send reviews because then that gives you a green light?

I expect people to be clear about what they suggest as later improvements. 
I've been trying to do so myself.

And not insist on them multiples times like you and others did. Especially not 
after you've already been told that it would be considered at a later time, or 
that we do not agree with the idea.

> Explicitly prefix most of everything with "not a blocker, but..." ?

I've actually been doing that sort of things several times lately when 
reviewing other people's code.

> Also, all three versions of your patchset had the module enabled by
> default, yet it was disabled in the version you merged:

You conveniently omit that I did send an email about that and other changes 
earlier, well before pushing.

-- 
Реми Дёни-Курмон
http://www.remlab.net/





More information about the vlc-devel mailing list