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

[Pierre Ynard]

> If I'd sent version 4 for review, here's where we'd be at:
> - Steve would still be complaining about logs and strings.
> - Pierre would probably complain about user-agent.
> - François and you would complain about code duplication and insists
> on using the first JSON API or updating the fingerprint code.
> - In all likelihood someone would question using JSON.
> - Likely somebody would complain about using Flex and Bison, because
> why not write a grammar directly in C when you have tools to generate
> the C code.
> And who knows when this would actually be merged, if ever.

Please assume good faith. Please don't assume what people would have
replied, and please don't assume that reviews are meant as complains
like you say.

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.

> 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? Send only positive reviews
because bad reviews are mean, annoying, and possibly misguided?
Explicitly prefix most of everything with "not a blocker, but..." ? Read
carefully the whole threads before voicing an opinion? Really, tell us
what you wanted or expected, and what you want now.

Also, you're getting remarks about the duplicated JSON parser now
anyway, that you still have to deal with - and they come off worse now
after the merge than they would have before.

When the discussion died down, I went ahead and told you that I was
fine with merging the patchset. That's what I could do. Others may or
may not have done what they could differently. But if you want to focus
on what you can do about the situation, perhaps you could simply voice
more clearly your expectations about merging the patchset, and for
example ask explicitly if there are still issues that people consider
blockers, because you'd like to go ahead and merge some code, and then
work further from there.

Also, all three versions of your patchset had the module enabled by
default, yet it was disabled in the version you merged: it's great that
we agree about being careful for potential issues, but one thing you
could have done there would have been to communicate your opinion about
merging a disabled version as starters.

-- 
Pierre Ynard
"Une âme dans un corps, c'est comme un dessin sur une feuille de papier."