From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-4.0 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 6C2AD1F454 for ; Sat, 9 Nov 2019 00:03:30 +0000 (UTC) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:from:to:references:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=TUER427Lb8Qcqb9j //nrHNQPksH37+RwGwMhA3lPmUrgba3NR0Vy1UxbxC9JEIPcWxfHBel9sUahsl52 KxQJUqslXKJEupOuG+jzVgK451Z+tX7M/oVL5eHtJrREfMiQUrsSuyMAMQUTZLnC 54LA3PFffraagulEEGxS7Jl/aWk= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:from:to:references:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=ytvwlm09MwfkR9t7lAf0hr oTU0I=; b=pka8V8Ng7RXo6ujPtGjxgjYEIxq5eZ84IciVmBpgB6ajTk9XrTWIay vK6sOAbWbMd8CsBrMDtAl8xXtrJnniJrYpLAfXog9w2mmt2WtrQ/njycp4VaFBB1 rPM2ozvb7We6njTa5IbuJVBcqOYuj4OvvC109rbdIpW4kXrt48ORQ= Received: (qmail 41499 invoked by alias); 9 Nov 2019 00:03:28 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 41482 invoked by uid 89); 9 Nov 2019 00:03:27 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: smtp.polymtl.ca DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca xA903Ifm018022 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1573257804; bh=CoqlrMTeAAuxC5eN1mdFsvQ0fVQOD/q2mrFVAfKVgJo=; h=Subject:From:To:References:Date:In-Reply-To:From; b=NxsoXfoaZKdtkf3ZEWU2th/4G9jeV0wMGLEDjv7RYbWPzm7lcrIRyGNVlwUc21gNS V+rIA37ip7BvZpBwob0q1hjt0sMXjgAexrbkSicoLaFGc6JmHY1oNz729c1L4ybcMq F9M91ZEbOpd9Fy2lIKI6KG2Ms8f4f6uaAF5abtdI= Subject: Re: Gerrit update - diff in comment notification emails From: Simon Marchi To: libc-alpha , "gdb-patches@sourceware.org" References: <326a619f-852b-5356-1e32-4dc4c6f833c1@polymtl.ca> Message-ID: <1ff70cda-7c39-cc3b-8244-8f9aab0bac26@polymtl.ca> Date: Fri, 8 Nov 2019 19:03:17 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <326a619f-852b-5356-1e32-4dc4c6f833c1@polymtl.ca> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 2019-11-08 12:16 p.m., Simon Marchi wrote: > Hi! > > In the last few days, I have tackled the problem of making the Gerrit > comment notification emails show comments in a diff context. > > Previously, we would show some context, but that context would only > include lines from the version of the file that was commented on. This > means that if the commenter had put a comment on the left pane in the > web UI, the comment would be placed in a context containing lines from > the "before" version of the file. If the commenter had put a comment on > the right pane, the context would show lines from the "after" version of > the file. > > This was already an improvement compared to Gerrit's normal behavior of > showing just one line of context (or the multiple lines of the range, > if the comment was placed on a range), but it still was lacking the > information to understand what was actually changed. > > The new format shows a portion of the diff around each comment, > resulting in something like this: > > https://sourceware.org/ml/gdb-patches/2019-11/msg00239.html > > I made the output look as much as possible like "git diff" output to > make people feel at home, but we are not limited to that. I hope that > this is a welcome change. If you have some ideas on how to improve it > further, I would be happy to hear your suggestions. I am sure we will > hit some corner cases for which the script doesn't give a good output, > so please report any problem you see. > > This is all implemented using a Python script [1] that uses Gerrit's > REST API, so the idea is that if you'd like to propose something, you > can actually try it out yourself and propose a code change. You can run > the script on any batch of already published comments to see what the > result would be. > > If the generated email doesn't contain any of the comments on the code, > such as this: > > https://sourceware.org/ml/gdb-patches/2019-11/msg00238.html > > it's probably because the script failed. It's then possible to just > run it locally, point it to the change, and see what happens. > > Note that while this shows the comments in the context of the diffs, it > doesn't show the comment you are replying to, if you were commenting in > reply to another comment. So if we just reply "Done" in the web UI, we > will only see "Done" in the notification email, which is not very > enlightening. So please try to use "Quote" instead of "Reply" and quote > the relevant portion of what you are replying to (just like you would by > email), so that it appears in the notification email. > > Simon > > [1] https://gnutoolchain-gerrit.osci.io/r/gitweb?p=gerrit.git;a=blob;f=resources/com/google/gerrit/pgm/init/generate-comment-diff.py;hb=refs/heads/stable-3.0-gnu I was told that this link was not accessible. It was a permission problem with Gitweb, it should be fixed now. Sorry for the trouble. Simon