On Mon, 04 May 2020 21:25:16 +0200, FRÉDÉRIC RECOULES wrote: > > Hi Takashi, > > I would like an update on the review process ([PATCH */6 V2] [pcm_dmix > assembly]) > > As a reminder, I split the changes in 6 distinct patches. > The 3 first patches produce exactly the same binary output, so they do not > need testing. > The 4th one has just some minor change due to the fact that I added an > instruction -- I am highly confident it breaks nothing. The compile tests passed with a few different compiler set, so that's good. But there were some issues with the patch format. IIRC, the patch 2 couldn't be applied to the latest git tree cleanly (some space letter issues?), so I had to manually modify it. Also, some style issues: - Please avoid a prefix like "[configure]" in the subject. The prefix with "[PATCH xxx]" is good, and this should remain, but the next prefix should be "configure:" instead. Otherwise the prefix with the brackets are pruned at applying a patch via git-am. - Please give more texts about why the change is done. In all your patches, there are no explanations why you change it. It's often more important than describing what you're changing. For example, the patch 2 "change the token by symbolic names". Why is this needed to be symbolic names? Write some more information in each patch description. - We usually use #ifdef without space between "#" and "ifdef". Let's keep that style consistently. > If you need I test the 2 last ones (that reduce the size of the produced > binary), could you point me out what test I should run? We need at least some build tests with different compiler versions and check whether dmix actually works (not necessarily on all of them but some of those compiled results). > Meanwhile, my deadline comes and I would really appreciate to see the patches > applied by wednesday night. If you can work on the above and resubmit v3 patchset, I'll happily apply them. Thanks! Takashi
