It was <2014-12-10 śro 10:42>, when Dominig ar Foll (Intel OTC) wrote: > I see today in Gerrit a patch pushed in an unacceptable way and I > would like to see that type of behaviour changed. > I speak about the review > https://review.tizen.org/gerrit/#/c/31701/ > > The push is justified on the comment : > > "Patch Set 3: > As of now, I feel adding these credentials here is the right way to do > because: > We want kdbus to be part of official images, so it is not going to > be optional, although we may disable kdbus at boot time by default. > Until we enable sysusers feature of systemd we'd like to avoid > adding and removing users with useradd/userdel." > > The "We want Kdbus ..." is not an agreed general feature
I beg to differ: https://wiki.tizen.org/wiki/Tizen_3.0 https://bugs.tizen.org/jira/browse/TC-1024 Please also note that I haven't indicated any timeframe we want kdbus running on the image. > In order to get it in, the same person pushed it, verified it, and > accept it in less than 2 hours of working day time in Europe where are > located the Common reviewers. > Sorry but that is not acceptable. I am afraid it's been more than 2 hours, please look at the comments once again. > 1) Same person submitting, verifying and accepting a patch (even > simple) is not in line with Tizen review model, You are right and I share your concern. > 2) A decent time should be left for reviewer to voice their > concerns. Typical 24h to cover multiple time zones. That's why I waited 42h befor putting +2 myself and still not merging. > 3) Architecture change shall be agreed by architecture team to get in > Common. Adding an entry for a user and a group in databases is hardly an architecture change, merely a preparation which in no way cannot, to the best of my knowledge, disrupt current operation. On the other hand a change like this, that a) has been checked by two independent developers (Philippe and José) b) got +1 from one of them, c) does add two lines (and changes one to make it look prettier), d) does not change absolutely anything in the way current system works, in my humble opinion can be handeled with a *little* less involvment From other developers. If you look around the Gerrit you will find that we work like this every day. Just a few examples from the top of the list: https://review.tizen.org/gerrit/31550/ https://review.tizen.org/gerrit/31314/ https://review.tizen.org/gerrit/31795/ https://review.tizen.org/gerrit/31787/ Is it bad? Not necessarily because right next to the rules you've mentioned is another value I hope we all share, it is trust. We trust that guys on the other end of the world know what they are doing and do not *always* bother them with pointing out every single formal mistake they make (I do, *sometimes*, complain about commit messages). In case of small changes like this one an oportunistic approach may be more efficient because the cost of a very through review is larger than cost of fixing a mistake multipled by probability of making it (risk). > So please do not force changes any more. > In that specific case, if you need Kdbus in a profile before than it > is agree to be generalised in Common, please make an agreement with > Common to enable it with a clean model. > In that specific case the use of a "%bcond_with kdbus" seems a viable model. I did exactly this in systemd-216 because it was the right way. I didn't do it here in the first place because it makes the patch twice as big, not a bit more readable and does not change anything in any other part of the sysem. > Please accept my apologies for having to be a bit rude. Accepted. Kind regards, -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics
pgpPv5KH8FrR2.pgp
Description: PGP signature
_______________________________________________ Dev mailing list Dev@lists.tizen.org https://lists.tizen.org/listinfo/dev
