Hi Tom, Andreas, On 7 August 2016 at 14:07, Tom Rini <tr...@konsulko.com> wrote: > On Sun, Aug 07, 2016 at 06:46:37PM +0200, Andreas Färber wrote: >> Am 07.08.2016 um 15:31 schrieb Tom Rini: >> > On Sat, Aug 06, 2016 at 06:05:29PM +0200, Andreas Färber wrote: >> >> Hi Simon, >> >> >> >> Am 06.08.2016 um 06:30 schrieb Simon Glass: >> >>> Hi Andreas, >> >>> >> >>> On 17 July 2016 at 19:06, Andreas Färber <afaer...@suse.de> wrote: >> >>>> Hi, >> >>>> >> >>>> This series adds initial support for RK3368 SoC and GeekBox. >> >>>> For more details see the commit message. >> >>>> >> >>>> Will need to be rebased onto Heiko's cleanups and Kever's RK3399 series. >> >>>> >> >>>> Regards, >> >>>> Andreas >> >>>> >> >>>> Cc: Simon Glass <s...@chromium.org> >> >>>> Cc: Kever Yang <kever.y...@rock-chips.com> >> >>>> Cc: Heiko Stübner <he...@sntech.de> >> >>>> >> >>>> Andreas Färber (2): >> >>>> dts: Import rk3368-geekbox.dts >> >>>> ARM64: rockchip: Add initial support for RK3368 based GeekBox >> >>>> >> >>> >> >>> Are you planning to respin these patches? >> >> >> >> Eventually...? >> >> >> >>> I'd like to get them applied soon. >> >> >> >> And I'd like to get my work recognized! However, despite our previous >> >> IRC chat, I had to find out _while_ replying to the rk3399 mails that >> >> you had once again not just applied all patches (twenty minutes after >> >> ack'ing them on a Saturday) but already sent a pull on Tuesday my >> >> nighttime that I was not CC'ed on and that Tom has merged the night >> >> after. So it feels like I'm wasting my time here and consequently I >> >> stopped my review and rebase. >> > >> > In the U-Boot community, we are not in the habit of cc'ing everyone with >> > a change in a given pull request. Is there a tool the kernel folks use >> > here that makes this easy? >> >> Not that I'm aware of. >> >> But that is besides the point, as my very complaint is that I'm not >> credited in the patches that got merged, so no tool could've extracted >> my name for CC'ing. > > Alright. > >> It's about Simon having mismerged those patches and having overlooked >> unresolved review comments of mine for those patches before and me >> specifically having complained to him about not waiting for my >> Reviewed-by before applying them. So him seeing that I did not reply to >> his Saturday mails, I feel it would've been fair in this particular case >> a) to ping me again after the weekend and b) to let me know that he is >> no longer waiting for my review comments or that I really need to hurry >> up with an objection until X. He did not say so in a reply that reached >> my inbox, and I was not CC'ed on the pull request itself, thus a pull >> request behind my back. >> >> I'm not too deep into U-Boot, so maybe there was a reason for this >> hush-hush workflow, but then at least the communication was fairly bad. >> >> Had I known that the pull is already on the list, I wouldn't have >> replied with a Reviewed-by for 1/4 that same day (which surely Simon was >> CC'ed on) or I could've asked Tom to hold off merging it until I'm done >> reviewing the next day. >> >> > And the rule of thumb that I use, and I try and get everyone else to use >> > as well is that a patch should be out for a week before it gets picked >> > up and merged as that should give everyone time to review, comment and >> > test. Did that not happen with the patches Simon picked up? >> >> Slightly less than a week. For some other projects it's ~two weeks. >> Also again note that this is not about some random patch but one where >> Simon specifically said he would be away, that he would exchange the >> patches on his branch where necessary and where he asked me to "sing". >> It leaves a bad taste that Simon was absent himself the week the patches >> were posted but apparently expects me to be available whenever he is. I >> don't work on U-Boot as a job, and for rebasing rk3368 patches - which >> many of my review comments resulted from - I need access to the hardware >> for testing. >> >> Note that I was similarly surprised how quickly two patches of mine went >> into his tree, with just one day in between and despite conflicts >> between my rk3368 and Kever's rk3399 preparations. I can see that having >> patches in a tree facilitates testing, but it also prevents serious peer >> review when not just staging but also merging them. > > I want to treat all of the above at once. First, sorry. We don't have > an intentionally "hush-hush" workflow, but every custodian does decide > how many emails to send when moving a patch forward. And unless I'm > testing multiple PRs (or they come in while I'm already testing one) the > time between getting a PR and applying it is usually the same (US, east > coast) day if it passes my testing. But we are trying to include more > crediting, not less, so it is not intentional to have left things > out[1]. So this was a mistake in our part, sorry. Sometimes review > comments are missed. But this too is not usually intentional unless > it's small things that can be addressed in a follow-up in order to get > things otherwise in and unblocking other work. In the end however, > Simon, please slow down a bit. It's OK if stuff misses a release, > there's always the next one. And it may make sense to follow what Hans > has been doing with the sunxi tree and having a next branch (which can > always be rebased and stuff replaced in!) so it's both clear that > patches are being picked up, but not rushing to get things in before a > window closes. Thanks!
OK thanks Tom for looking at this. I'll slow down a bit on the Rockchip stuff. I'll look at using a -next branch for that also. Andreas let me know how things go. I'd really like to see a review for new patches within a week if possible (absent holidays, etc.). > > -- > Tom > > [1]: Patchwork had an outage recently and some stuff didn't make it in > there and thus was missed unless picked up manually. Most tags come in > via patchwork not manually adding to the commit message. Yes that was a pain! Regards, SImon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
