From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id r8Z+OrV9imcudRIAWB0awg (envelope-from ) for ; Fri, 17 Jan 2025 10:56:37 -0500 Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=BddtCXPO; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id DE8C31E100; Fri, 17 Jan 2025 10:56:37 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-5.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=unavailable autolearn_force=no version=4.0.0 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 03C4B1E05C for ; Fri, 17 Jan 2025 10:56:37 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 816F2384772D for ; Fri, 17 Jan 2025 15:56:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 816F2384772D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1737129396; bh=0IlwcUEnrqAmiaNWRrg4CdSTndKdf+vtZByklcso4SI=; h=Date:Subject:To:Cc:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=BddtCXPOovzxkMOJetJK7MnboeWHcsq7uQQSiEVnRYvMTzqYB04ATRDl57RX5HTr/ G2vuajhQFyODsON6P2fa2sHbtjk5LpNftJuFuXaAMmAxdQSdNcUM3D5pv9eR561xwY JPN1TcY+n7jI3BGLtV8Yd22l5I1x094jP9y+Mias= Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id E7B52386F01A for ; Fri, 17 Jan 2025 15:55:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E7B52386F01A ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E7B52386F01A ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1737129349; cv=none; b=Ub+RUlKsB7AamwjDhjISZkiEugLqXGspTxA+bTwfR51smbRlKartMOJDyrpXkSyFy3bW8TLhLf6EIWJnMSKSiyTk+IpIdkQGySTXqLQj3CAH1rwUCQ+ZJHJRdrqJrJwZVi6br+C8kmOo6pWpva4ohbS4FpORHDjj7rslBVC2ibk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1737129349; c=relaxed/simple; bh=rf1OjdTyqdEqLJjQHjboA8K7zXzYxrkQdFidHrYx4lU=; h=DKIM-Signature:DKIM-Signature:Message-ID:Date:MIME-Version: Subject:To:From; b=J/mWcb/nD4aVjgDPCu3q3c7GF9btRdRMA3HY090VHVZBT2osnuSSeRnmlc1hUMlHBzk7J84YbVrd3yDTI7T+l+T+zZRPAWafdxawWlOqOwsFlqCCkgv7+Hpmz73nQimjtOUp5xMtoxEQzYgFoPc58NgyIjbft/AHeQjNSesOI9c= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E7B52386F01A Received: by simark.ca (Postfix, from userid 112) id 6CDBA1E05C; Fri, 17 Jan 2025 10:55:48 -0500 (EST) Received: from [192.168.126.187] (unknown [204.48.78.22]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 415201E05C; Fri, 17 Jan 2025 10:55:45 -0500 (EST) Message-ID: Date: Fri, 17 Jan 2025 10:55:44 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: automated coding style tool To: Joel Brobecker , Andrew Burgess Cc: Luis Machado , Tom Tromey , "Aktemur, Tankut Baris via Gdb" , "Aktemur, Tankut Baris" References: <74c8b867-f5bb-48f7-9849-11d06e63a3d7@arm.com> <87tta2r5z2.fsf@redhat.com> <87tta1qq1i.fsf@tromey.com> <2985f4ae-33c0-4e02-8982-fa132f4d3741@simark.ca> <87msftuhd3.fsf@tromey.com> <877c6to8um.fsf@redhat.com> Content-Language: fr In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: gdb@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Simon Marchi via Gdb Reply-To: Simon Marchi Errors-To: gdb-bounces~public-inbox=simark.ca@sourceware.org Sender: "Gdb" On 1/17/25 10:13 AM, Joel Brobecker wrote: >> I don't think adding this tool is going to be the magic bullet everyone >> seems to think it is. >> >> As a reviewer you'll either still need to learn the style in order to >> spot when a contributor has not running the formatting tool. Either >> that, or you need to apply each patch locally and the check run the >> formatting tool to check the formatting is correct. > > I have experience with a project where developers were asked to install > a pre-commit check that would run the formatting tool, but where we did > not have (yet) a pipeline to verify the formatting at submission time. > This project had a lot of developers, some of them very occasional. > > It's true that, at first, we had a number developers forgot to install > that pre-commit hook, and ended up submitting ill formatted code. > > What I noticed, at least for myself, is that with time you start > developing an eye for what the expected formatting is like. And when > I noticed something odd, I'd just ask if they installed the pre-commit > and ran the formatting tool. If they say no, I pointed them to > the directions, and asked them to retest and resubmit. Ultimately, > we mostly converged. > > And for the cases we didn't notice at review - no big issue. > The formatting shouldn't be horrible of the reviewer would have > noticed it, so no great harm -- certainly no worse than right now. > And later on, the next developer touching the same file ends up > noticing that the formatting seems wrong. It's a bit of a pain > for them to first run the formatting tool and push a pure-reformatting > commit. But we tell the contributor who forgot, they install the tool, > and normally we eventually converge. > > Another things we can imagine doing is a nightly job on sourceware > that runs the nightly snapshot through the formatting tool, and > sends an email with a list of files that are not conformant. > > Where I think it might be painful is which version of the tool > to use. Different versions will likely have slightly different > results in terms of formatting the code. You might want to require > a specific version and check for that version, to avoid different > users having different ways of formatting the same code. I agree with all the above. We will definitely want to choose a specific version of the tool. If we end up using clang-format, that version will likely not be available out of the box for all distros, but there are other ways to get it. I don't know how it works on Windows though, since I don't develop on Windows much. We can cross that bridge when we get there. >> I think if GDB could just move away from the mailing list and each users >> pushes their own patches model, over to a pull request style approach, >> then we could potentially have _real_ automated checks, e.g. checking >> the formatting. Then we have an _actual_ win. > > Seconded. > > In a model like this, you don't have to ask. You run a job that > checks it, and you know if it's conformant or not. I don't agree with Andrew, in that even without a proper pre-merge CI, you do save time by using a tool to format the code, over doing and reviewing it manually. Of course, like Joel said, ill-formatted code will certainly get committed at some point, but the nightly job will tell us. It's a quick fix - one person pushes an obvious patch - and we carry on. Much less time overall than pointing out formatting issues one by one (for the reviewer) and fixing them by hand (for the author). This is what we do with our Python code and it works well, I think. I suppose it would also be possible to write a server-side commit hook that checks whether the formatting of the affected files is correct, and reject the push if not. Simon