From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: New Git hooks for checking file names in commit messages Date: Sun, 23 Apr 2023 09:11:29 +0300 Message-ID: <83v8hnyvou.fsf@gnu.org> References: <838rel46ou.fsf@gnu.org> <865y9ptfa3.fsf@aarsen.me> <83354t44kn.fsf@gnu.org> <875y9p2h3u.fsf@thaodan.de> <83h6t82yao.fsf@gnu.org> <0a76fcd4-df69-56e0-ba90-30dc211ad56e@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="19849"; mail-complaints-to="usenet@ciao.gmane.io" Cc: bjorn.bidar@thaodan.de, arsen@aarsen.me, acm@muc.de, emacs-devel@gnu.org To: Jim Porter Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Apr 23 08:12:11 2023 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pqSxH-0004xY-8o for ged-emacs-devel@m.gmane-mx.org; Sun, 23 Apr 2023 08:12:11 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pqSwQ-0005dr-6z; Sun, 23 Apr 2023 02:11:19 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pqSwL-0005dh-To for emacs-devel@gnu.org; Sun, 23 Apr 2023 02:11:14 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pqSwK-0004V9-AG; Sun, 23 Apr 2023 02:11:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From: Date; bh=Jz9TZTVQgtfwJVW/8jrlV+KD+Uah91s6oR3rwHE0gtQ=; b=DsRp0bihQAVNXZaWFxw1 7mRP96FGY7kQ7B4cXCkRzPj+h2KE8km5Tc99oCRpWSHxE5Ipxqbp3jj5d5Xz5Q5plYnSzIbM/XRCw VGIxnnygvhY/QicZpTTto5SXuIGwVhxC/adl79IyEM/201/uPZ6+JbxPTdsB56TLPRUdLMapXrEUn h0Oh3fhgfcBBKTqL81jpJnPzkz9qfN7wkLkhB7N9zRQ6DqvseOHwayTwT+XyXBtUZWxjKdwBFKI0S /fqeDvcGlxHctldiHY80YWoEixXcDCjV4BlOyocu1UbS1PZ3cOkuVyHC/E4qyfFw8eGgQ9P07Gtch eY46WiaIlUCLXw==; Original-Received: from [87.69.77.57] (helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pqSwI-0001Nv-62; Sun, 23 Apr 2023 02:11:11 -0400 In-Reply-To: <0a76fcd4-df69-56e0-ba90-30dc211ad56e@gmail.com> (message from Jim Porter on Sat, 22 Apr 2023 12:52:58 -0700) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:305596 Archived-At: > Date: Sat, 22 Apr 2023 12:52:58 -0700 > Cc: arsen@aarsen.me, acm@muc.de, emacs-devel@gnu.org > From: Jim Porter > > On 4/22/2023 12:03 AM, Eli Zaretskii wrote: > >> From: Björn Bidar > >> Should branches be checked by these hooks before they are merged into > >> master/version branches? > > > > No. We don't enforce these rules on branches from which Emacs > > tarballs will not be produced. The commit log message which matters > > for those is the one of the merge-commit when the branch is landed on > > master. > I'll make sure this works. I do think it's useful to check commits on > all branches, since we won't know until the merge how a committer plans > to do the merge. The risk exists, but the freedom we give developers with log messages on feature branches is more important. We should not make developing features more troublesome by enforcing well-formatted log messages for WIP changes, especially since frequently the commits are just checkpoints, they don't signify any changes that have a useful description. So we should not block pushing of branch commits due to log messages. > For example, you could push a feature branch upstream, and when the > time comes to merge it, rebase it onto master. In that case the > history is linear, so we'd want to check every commit that was > newly-added to master. We prefer that people do not rebase onto master. We hope they don't do that, in practice. They can rebase the branch before merging it, of course, but that is not the situation you describe, and should not cause problems with these hooks, because only the log message of the final merge-commit matters. > I'm not exactly sure how the Git log -> ChangeLog -> etc/AUTHORS process > works for merge commits Merge commits usually don't have any meaningful log messages and don't mention files. You must explicitly add a log message mentioning files and functions for a merge-commit to have a log message, and we request that people who land feature branch create a log message describing all the new/changed things in the merge for that single commit before you push. Most other merge-commits aren't important in the context of this discussion. > For example, we could just detect if we're pushing a merge commit > and then stop validating any commits-to-be-pushed prior to that. If this can be done easily and without mistakes, that would be okay, I think. But please consider all the frequent use cases where merge-commits appear. These include: . merge-commit by "git pull" when there are local commits (several people making changes to the same branch at the same time) . merge from the release branch to master . cherry-pick from master to the release branch . merge of a feature branch or a scratch branch > Of course, we could also say that it's still useful to validate all > those commits anyway, since it keeps our commit history better-formatted. But if so, what do you want the user who does the merge do? The log messages of past commits cannot be amended, so this would simply preclude people from pushing merges and/or force them to use the "--no-verify" switch. For example, a frequent situation for merges is a merge from the release branch to master -- if one or more of the commits being merged has bad log messages, we don't want to block the merge, because nothing can be done about those bad messages at the time of the merge. > If there's ever a case where these new hooks do the wrong thing, we can > just run "git push --no-verify" to ignore the hooks. We're the humans, > so we have the final word over the computer, after all. :) Increasing the number of situations where we'd need --no-verify is not a Good Thing. In this case, it is better to leave the badly-formatted log messages alone, and let them be dealt with at make-tarball time.