[FFmpeg-devel] [PATCH 1/2] lavu/dict: Add new flag to allow multiple equal keys.

[wm4]

On Sat, 26 Mar 2016 01:12:17 +0100
Thilo Borgmann <thilo.borgmann at mail.de> wrote:

> Am 25.03.16 um 21:12 schrieb wm4:
> > On Fri, 25 Mar 2016 19:41:40 +0100
> > Thilo Borgmann <thilo.borgmann at mail.de> wrote:
> >   
> >> Am 25.03.16 um 18:48 schrieb Hendrik Leppkes:  
> >>> On Fri, Mar 25, 2016 at 6:26 PM, Thilo Borgmann <thilo.borgmann at mail.de> wrote:    
> >>>> Am 25.03.16 um 17:56 schrieb Hendrik Leppkes:    
> >>>>> On Fri, Mar 25, 2016 at 5:48 PM, Thilo Borgmann <thilo.borgmann at mail.de> wrote:    
> >>>>>> Am 22.03.16 um 12:20 schrieb Thilo Borgmann:    
> >>>>>>> Am 22.03.16 um 11:45 schrieb wm4:    
> >>>>>>>> On Sun, 13 Mar 2016 21:00:23 +0100
> >>>>>>>> Thilo Borgmann <thilo.borgmann at mail.de> wrote:
> >>>>>>>>    
> >>>>>>>>> Am 13.03.16 um 15:08 schrieb wm4:    
> >>>>>>>>>> On Sat, 12 Mar 2016 15:13:21 +0100
> >>>>>>>>>> Thilo Borgmann <thilo.borgmann at mail.de> wrote:
> >>>>>>>>>>    
> >>>>>>>>>>> From a1d9ce388c69eabb256e6b351c2acd36d7f4076e Mon Sep 17 00:00:00 2001
> >>>>>>>>>>> From: Thilo Borgmann <thilo.borgmann at mail.de>
> >>>>>>>>>>> Date: Sat, 12 Mar 2016 14:52:17 +0100
> >>>>>>>>>>> Subject: [PATCH 1/2] lavu/dict: Add new flag to allow multiple equal keys.
> >>>>>>>>>>> [...]    
> >>>>>>>>>>
> >>>>>>>>>> Changing the semantics of AVDictionary just like this seems rather
> >>>>>>>>>> questionable...    
> >>>>>>>>>
> >>>>>>>>> It changes nothing for existing code, just adds a new feature. I don't
> >>>>>>>>> think it hurts anyone or anything...    
> >>>>>>>>
> >>>>>>>> It only breaks basic assumptions about a basic data type...    
> >>>>>>>
> >>>>>>> Although I don't share your thought about breaking a basic data type with that,
> >>>>>>> what would you suggest instead?    
> >>>>>>
> >>>>>> Pushed for no further suggestions and nobody else objected.
> >>>>>>    
> >>>>>
> >>>>> Just pushing without addressing concerns is not the way we usually try
> >>>>> to work here, just saying.    
> >>>>
> >>>> I think I have quite a good idea about the usual way we try to handle
> >>>> things here and I think I've addressed his concerns.    
> >>>
> >>> If by addressing you mean disagreeing with the concern and doing nothing.    
> >>
> >> Not at all. I proposed alternatives (alternatives which I don't like
> >> much but anyway) and I explicitly asked for his suggestions. Means, I
> >> actually tried to satisfy his concerns. Thus, I can't understand that
> >> you are saying I've ignored anything.
> >>
> >>  
> >>>> He didn't like it which is of course ok. He did not continue discussing
> >>>> it nor did he proposed any alternative. He also did not pick up any of
> >>>> my thoughts. He also did not explicitly state that he thinks that it is
> >>>> not ok to apply it. He said it "seems rather questionable". Without
> >>>> further discussion (what I tried) and nobody else complaining about it,
> >>>> what do you think would be more appropriate than to wait for quite a
> >>>> long time until continuing?
> >>>>
> >>>> The usual way for him to prevent me pushing it would just have been to
> >>>> ask me to wait and I would have waited. Have you checked the dates of
> >>>> the replies and what I wrote before accusing me to just ignore concerns?
> >>>>    
> >>>
> >>> Timing makes no difference. Its the only review you got, so even if
> >>> you ignore that, you didn't even get a "OK" from anyone else, which
> >>> for generic API should be mandatory.    
> >>
> >> I can't see why you accuse me ignoring something again.
> >>
> >>  
> >>> The least that would have been appropriate would be to ping the patch
> >>> asking for further comments, instead of just practically saying "I'm
> >>> done waiting and just pushing"
> >>>
> >>> Not everyone has the time to answer within a day, so if someone
> >>> expressed a concern, the least one could do before pushing is asking
> >>> again, everything else feels rather disingenuous.    
> >>
> >> First concern about this was stated on 13th.
> >> After my reply, there was silence for nine days.
> >> What would have been your assumption about his concerns after my reply?
> >> Mine was that he considers this not to be as critical enough for further
> >> discussion - means he might still dislike having multikey dictionaries
> >> but sees no reason in struggling about it.
> >>
> >> I pinged the patch again on 22nd, and it took about one minute for wm4
> >> to address his concerns again. However, after me asking for his
> >> suggestions there was again silence for days. Also note that he did not
> >> stated his concerns more specifically than before.
> >>
> >> So I waiting for around 12 days (including a ping) to get a more
> >> specific remark, counter-proposal, discussion or anything else than a
> >> basic concern. During that time wm4 was active and very well capable of
> >> immediate reply. Thus I assume that his attitude about this patch is not
> >> as bad as insisting not to apply.
> >>
> >> I still really can't see a "I'm done waiting and just pushing" attitude
> >> from my side.  
> > 
> > You were adding weird new public API just to internally parse some
> > really weird syntax. I hoped other would voice their concern too, but
> > nobody did, so who cares, I guess.  
> 
> Which means my assumption about your attitude to this patch was not
> totally wrong. Also you did not reply at all when I asked for your
> suggestions.

I don't particularly care for this issue (which is why I've mostly
remained silent). I just wish we wouldn't change the public API in
frivolous and tricky ways (such as suddenly allowing duplicate keys)
without much discussion.

> Which leads to a silent approval.

Not at all.