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 AC4FC1F454 for ; Fri, 8 Nov 2019 17:16:39 +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:to:from:subject:message-id:date:mime-version :content-type:content-transfer-encoding; q=dns; s=default; b=VUb NLP3jDZfwjHyxJNvOnluP0YUs0LumxvQKFQ2Z4OJTSs41rc1Pi0jbECuroBFlUMz 6EfjyAz6Je3x8+8689vKDUcmk1kH8j/dUWvCufEqWdsBxHWPPa4qHmaWG3F9oDR1 yvXlE1KNtXTYCq9lZpG/0BbhKAfdYkyfBqYYyQo8= 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:to:from:subject:message-id:date:mime-version :content-type:content-transfer-encoding; s=default; bh=jdzEWlgfQ Ta4JuOYjFlQ4qCFf8w=; b=O2u23p6gNKPJPJbY0MOhN3HvSWI+KOCT9v3AvYdZA auXvOx/vUIDM3BSQGME8CQBrAf0qiLyr/2r5N1WKHGs9Tw+DZxrkRfvaxtiiwDqe 9eoLvpOyFq/EPVmIU7Jy0mF7jtOJKIeqY1snPoyahYl0T3COImKDrRN0jMmEsMfx Ds= Received: (qmail 90057 invoked by alias); 8 Nov 2019 17:16:37 -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 90041 invoked by uid 89); 8 Nov 2019 17:16:37 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: smtp.polymtl.ca DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca xA8HGRfe003135 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1573233393; bh=kXMbzg8Nn/HO747rh2b+Q5C4eDRQGfGs2PO+jKJSkkM=; h=To:From:Subject:Date:From; b=JHC0r1NH2En3QSYDeyFPE8yRAiimMQhxDqIh/0krBis9XfCMtPv2fDNLOhypcCQ+h KBXAsOF/N4jAHw3qWxUjC6PY3DVq/SY8jL8pJ1N1EJ6AjNvX7R4S03kASM5B2AxDnd GEaaXgF3y5DdS4kQAmMeH7PzvODGKuGAanxDQX40= To: libc-alpha , "gdb-patches@sourceware.org" From: Simon Marchi Subject: Gerrit update - diff in comment notification emails Message-ID: <326a619f-852b-5356-1e32-4dc4c6f833c1@polymtl.ca> Date: Fri, 8 Nov 2019 12:16:26 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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