On Mon, May 25, 2020 at 9:22 PM Maxime Coquelin <maxime.coque...@redhat.com> wrote: > > > > On 5/25/20 5:35 PM, Jerin Jacob wrote: > > On Mon, May 25, 2020 at 8:52 PM Thomas Monjalon <tho...@monjalon.net> wrote: > >> > >> 25/05/2020 16:28, Burakov, Anatoly: > >>> On 25-May-20 1:53 PM, Thomas Monjalon wrote: > >>>> 25/05/2020 13:58, Jerin Jacob: > >>>>> 25/05/2020 11:34, Morten Brørup: > >>>>>> sending patches over an > >>>>>> email as opposed to a well-integrated web interface workflow is so > >>>>>> alien > >>>>>> to most people that it definitely does discourage new contributions. > >>>>>> > >>>>>> I understand the advantages of mailing lists (vendor independence, > >>>>>> universal compatibility, etc.), but after doing reviews in > >>>>>> Github/Gitlab > >>>>>> for a while (we use those internally), going through DPDK mailing list > >>>>>> and reviewing code over email fills me with existential dread, as the > >>>>>> process feels so manual and 19th century to me. > >>>>> > >>>>> Agree. I had a difference in opinion when I was not using those tools. > >>>>> My perspective changed after using Github and Gerrit etc. > >>>>> > >>>>> Github pull request and integrated public CI(Travis, Shippable , > >>>>> codecov) makes collaboration easy. > >>>>> Currently, in patchwork, we can not assign a patch other than the set > >>>>> of maintainers. > >>>>> I think, it would help the review process if the more fine-grained > >>>>> owner will be responsible for specific > >>>>> patch set. > >>>> > >>>> The more fine-grain is achieved with Cc in mail. > >>>> But I understand not everybody knows/wants/can configure correctly > >>>> an email client. Emails are not easy for everybody, I agree. > >>>> > >>>> I use GitHub as well, and I really prefer the clarity of the mail > >>>> threads. > >>>> GitHub reviews tend to be line-focused, messy and not > >>>> discussion-friendly. > >>>> I think contribution quality would be worst if using GitHub. > >>> > >>> I have more experience with Gitlab than Github, but i really don't see > >>> it that way. > >>> > >>> For one, reviewing in Gitlab makes it easier to see context in which > >>> changes appear. I mean, obviously, you can download the patch, apply it, > >>> and then do whatever you want with it in your editor/IDE, but it's just > >>> so much faster to do it right in the browser. Reviewing things with > >>> proper syntax highlighting and side-by-side diff with an option to see > >>> more context really makes a huge difference and is that much faster. > >> > >> OK > >> > >> > >>> I would also vehemently disagree with the "clarity" argument. There is > >>> enforced minimum standard of clarity of discussion in a tool such as > >>> Gitlab. I'm sure you noticed that some people top-post, some > >>> bottom-post. Some will remove extraneous lines of patches while some > >>> will leave on comment in a 10K line patch and leave the rest as is, in > >>> quotes. Some people do weird quoting where they don't actually quote but > >>> just copy text verbatim, making it hard to determine where the quote > >>> starts. If the thread is long enough, you'd see the same text quoted > >>> over and over and over. All of that is not a problem within a single > >>> patch email, but it adds up to lots of wasted time on all sides. > >> > >> Yes > >> > >> My concern about clarity is the history of the discussion. > >> When we post a new versions in GitHub, it's very hard to keep track > >> of the history. > >> As a maintainer, I need to see the history to understand what happened, > >> what we are waiting for, and what should be merged. > > > > IMO, The complete history is available per pull request URL. > > I think, Github also email notification mechanism those to prefer to see > > comments in the email too. > > > > In addition to that, Bugzilla, patchwork, CI stuff all integrated into > > one place. > > I am quite impressed with vscode community collaboration. > > https://github.com/Microsoft/vscode/pulls > > Out of curiosity, just checked the git history and I'm not that > impressed. For example last commit on the master branch: > > https://github.com/microsoft/vscode/commit/2a4cecf3f2f72346d06990feeb7446b3915d6148 > > Commit title: " Fix #98530 " > Commit message empty, no explanation on what the patch is doing.
Yes. The merging rules, how much review is required, sanity test to per check-in will be specific to the project requirements. I can see zephyr RTOS project(I am following this project) will be more close to the coding standard and other requirements. https://github.com/zephyrproject-rtos/zephyr/pulls > > Then, let's check the the issue it is pointed to: > https://github.com/microsoft/vscode/issues/98530 > > Issue is created 15 minutes before the patch is being merged. All that > done by the same contributor, without any review. >
