Message ID | 86wozwbv68.fsf@gmail.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 79787 invoked by alias); 1 Feb 2018 15:12:56 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 79778 invoked by uid 89); 1 Feb 2018 15:12:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=1446, Hx-spam-relays-external:209.85.128.196, H*RU:209.85.128.196, H*r:sk:static. X-HELO: mail-wr0-f196.google.com Received: from mail-wr0-f196.google.com (HELO mail-wr0-f196.google.com) (209.85.128.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 01 Feb 2018 15:12:54 +0000 Received: by mail-wr0-f196.google.com with SMTP id v15so19350475wrb.8 for <gdb-patches@sourceware.org>; Thu, 01 Feb 2018 07:12:54 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=j4FPhxBzmpE9mgsjlG1AP/wn5Nh/lZtHjcvwR37InqQ=; b=D5fa2I8Ab0xspWAwrahjdISYpcMRic2KAXDEKK2Slail0th8RBllL6FUJ54qMdK7WX VGAjwC4Vwu9QnN8ea3Z0I0cU2i8TphI1dY6W4eSPjUhvHag+fywYH8EQWdQ6dDxUcEVS ufwJ9DheB2pheWWrfi7qy/d4JlZSKHG4WDEjUgMT2CHPKVw2BygqRFolMmSNuCx9X6o9 lPIoDq8GaO8moPLUx3XPV9BOGGTcX05Vo7RhL2QkgZyCQ4tZiJ9AOHbppuZoQ5g5p9BU 9RiUasqFTY09mFckPQ21HHZZ1eWwXo9Tn5ZSmMHsdhli08CWEfep4aeH/BuVCbI/H6Br UzFA== X-Gm-Message-State: AKwxytevX5VoNSw4eBy1vqtOq6lm6+KWdXvbPHoYVs7FILNz4LQwjyk3 K3lyPUH/C6zibu80YAon2jwyPw== X-Google-Smtp-Source: AH8x226QklIIk6JGHFFrItYh2QUtVOIMPjhZNQj9EQPhMgt+vDVd5c5Nc+cXzDhi7z2dKio4YA5fWg== X-Received: by 10.223.184.28 with SMTP id h28mr22571488wrf.215.1517497972365; Thu, 01 Feb 2018 07:12:52 -0800 (PST) Received: from E107787-LIN (static.42.136.251.148.clients.your-server.de. [148.251.136.42]) by smtp.gmail.com with ESMTPSA id e6sm19681534wra.41.2018.02.01.07.12.51 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Thu, 01 Feb 2018 07:12:51 -0800 (PST) From: Yao Qi <qiyaoltc@gmail.com> To: Eli Zaretskii <eliz@gnu.org> Cc: simon.marchi@ericsson.com, gdb-patches@sourceware.org Subject: Re: [ANNOUNCEMENT] GDB 8.1 release branch created! References: <announce.20180105041805.3FC35808E9@joel.gnat.com> <83h8rlyakm.fsf@gnu.org> <83lggvupt6.fsf@gnu.org> <83lgglnadl.fsf@gnu.org> <83fu6sln4o.fsf@gnu.org> <3d75778f-5c91-5680-b9fa-c2f2c902ff67@ericsson.com> <838tcklaev.fsf@gnu.org> <83607njlsh.fsf@gnu.org> Date: Thu, 01 Feb 2018 15:12:47 +0000 In-Reply-To: <83607njlsh.fsf@gnu.org> (Eli Zaretskii's message of "Sat, 27 Jan 2018 18:42:22 +0200") Message-ID: <86wozwbv68.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes |
Commit Message
Yao Qi
Feb. 1, 2018, 3:12 p.m. UTC
Eli Zaretskii <eliz@gnu.org> writes: > No further comments, so I went ahead and pushed the change, without > the __cplusplus part, to master, and then cherry-picked to > gdb-8.1-branch. Hi Eli, Is it intended to include error messages in the changelog entry? If not, can we remove it? in the patch below, 2018-01-27 Eli Zaretskii <eliz@gnu.org> Avoid compilation errors in MinGW native builds The error is triggered by including python-internal.h, and the error message is: In file included from d:\usr\lib\gcc\mingw32\6.3.0\include\c++\math.h:36:0, from build-gnulib/import/math.h:27, from d:/usr/Python26/include/pyport.h:235, from d:/usr/Python26/include/Python.h:58, from python/python-internal.h:94, from python/py-arch.c:24: d:\usr\lib\gcc\mingw32\6.3.0\include\c++\cmath:1157:11: error: '::hypot' has not been declared using ::hypot; ^~~~~ This happens because Python headers define 'hypot' to expand t '_hypot' in the Windows builds. * python/python-internal.h (_hypot) [__MINGW32__]: Define back to 'hypoth'. This avoids a compilation error.
Comments
> From: Yao Qi <qiyaoltc@gmail.com> > Cc: simon.marchi@ericsson.com, gdb-patches@sourceware.org > Date: Thu, 01 Feb 2018 15:12:47 +0000 > > Eli Zaretskii <eliz@gnu.org> writes: > > > No further comments, so I went ahead and pushed the change, without > > the __cplusplus part, to master, and then cherry-picked to > > gdb-8.1-branch. > > Hi Eli, > Is it intended to include error messages in the changelog entry? If > not, can we remove it? in the patch below, Maybe I'm missing something, but isn't the commit log entry supposed to have the same text as the ChangeLog for that change? And what's wrong with having error messages in ChangeLog's, anyway?
On Thu, Feb 1, 2018 at 4:27 PM, Eli Zaretskii <eliz@gnu.org> wrote: > Maybe I'm missing something, but isn't the commit log entry supposed > to have the same text as the ChangeLog for that change? My understanding is that ChangeLog entry should be copied at the end of commit log https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_formatted_commit_messages > > And what's wrong with having error messages in ChangeLog's, anyway? Each entry in ChangeLog describes the changes to the code, https://www.gnu.org/prep/standards/standards.html#Change-Log-Concepts IMO, error messages are out of the scope of what ChangeLog is intended to describe. Also, I've never seen such ChangeLog entry (with error messages) before.
> From: Yao Qi <qiyaoltc@gmail.com> > Date: Thu, 1 Feb 2018 16:51:36 +0000 > Cc: Simon Marchi <simon.marchi@ericsson.com>, GDB Patches <gdb-patches@sourceware.org> > > On Thu, Feb 1, 2018 at 4:27 PM, Eli Zaretskii <eliz@gnu.org> wrote: > > Maybe I'm missing something, but isn't the commit log entry supposed > > to have the same text as the ChangeLog for that change? > > My understanding is that ChangeLog entry should be copied at the end > of commit log I believe I did that, except that I moved the motivation for the change before the ChangeLog header, as it belongs to the preamble of the Git log. (In ChangeLog, there's no subdivision of the entry.) > > And what's wrong with having error messages in ChangeLog's, anyway? > > Each entry in ChangeLog describes the changes to the code, > https://www.gnu.org/prep/standards/standards.html#Change-Log-Concepts > IMO, error messages are out of the scope of what ChangeLog is intended > to describe. This is changing, and some GNU projects already do it differently. I believe the next version of standards.texi will change the recommendations to be more lax. And using different style for each project is a pain. Is GDB really so rigid as to not tolerate this? Please note that the error message I cited is just part of the motivation for the change, so we are really talking about including the motivation in the ChangeLog. > Also, I've never seen such ChangeLog entry (with error messages) > before. That's not necessarily a reason for rejecting such entries. So: what is the project's take on this, if there is one?
On Thu, Feb 1, 2018 at 5:34 PM, Eli Zaretskii <eliz@gnu.org> wrote: >> From: Yao Qi <qiyaoltc@gmail.com> >> Date: Thu, 1 Feb 2018 16:51:36 +0000 >> Cc: Simon Marchi <simon.marchi@ericsson.com>, GDB Patches <gdb-patches@sourceware.org> >> >> On Thu, Feb 1, 2018 at 4:27 PM, Eli Zaretskii <eliz@gnu.org> wrote: >> > Maybe I'm missing something, but isn't the commit log entry supposed >> > to have the same text as the ChangeLog for that change? >> >> My understanding is that ChangeLog entry should be copied at the end >> of commit log > > I believe I did that, except that I moved the motivation for the > change before the ChangeLog header, as it belongs to the preamble of > the Git log. (In ChangeLog, there's no subdivision of the entry.) > It is absolutely fine to put the reason of the change in the commit log, but it is questionable to put commit log into ChangeLog. At least, nowadays, "commit log" is more than ChangeLog entry. This is the convention we followed for several years. >> > And what's wrong with having error messages in ChangeLog's, anyway? >> >> Each entry in ChangeLog describes the changes to the code, >> https://www.gnu.org/prep/standards/standards.html#Change-Log-Concepts >> IMO, error messages are out of the scope of what ChangeLog is intended >> to describe. > > This is changing, and some GNU projects already do it differently. I > believe the next version of standards.texi will change the > recommendations to be more lax. And using different style for each > project is a pain. Is GDB really so rigid as to not tolerate this? > GDB is evolving, so I am completely fine to changes, but you made the change of the way of writing changelog, which is not discussed before. > Please note that the error message I cited is just part of the > motivation for the change, so we are really talking about including > the motivation in the ChangeLog. > Yes, do we want to change the rule of writing ChangeLog, so that we can/should put the motivation for the change? Is the motivation mandatory in the changelog or optional? >> Also, I've never seen such ChangeLog entry (with error messages) >> before. > > That's not necessarily a reason for rejecting such entries. > > So: what is the project's take on this, if there is one? We need a clear rule on writing changelog, otherwise how do we write changelog or review patches?
> From: Yao Qi <qiyaoltc@gmail.com> > Date: Thu, 1 Feb 2018 21:32:13 +0000 > Cc: Simon Marchi <simon.marchi@ericsson.com>, GDB Patches <gdb-patches@sourceware.org> > > > Please note that the error message I cited is just part of the > > motivation for the change, so we are really talking about including > > the motivation in the ChangeLog. > > > > Yes, do we want to change the rule of writing ChangeLog, so that > we can/should put the motivation for the change? Is the motivation > mandatory in the changelog or optional? > > >> Also, I've never seen such ChangeLog entry (with error messages) > >> before. > > > > That's not necessarily a reason for rejecting such entries. > > > > So: what is the project's take on this, if there is one? > > We need a clear rule on writing changelog, otherwise how do we > write changelog or review patches? I agree. I hope Someone(TM) will answer these questions.
> > We need a clear rule on writing changelog, otherwise how do we > > write changelog or review patches? > > I agree. I hope Someone(TM) will answer these questions. Well, if it were me, I would stop using ChangeLog files with immediate effect. The purpose of ChangeLog files, as I understand it, is to describe the "what changed", and is aimed at people who download a given release and want to know what changed (because they do not have access to the version control information). While the above might have been essential 20 or 25 years ago, it is trivial for anyone today to have access to the change history of our program. Two options: - git clone our public repository; or... - ... if they don't want to use git, they can use the gitweb interface. I think that this is an acceptable way for us to provide the log of changes that were made. Currently, the situation is that we all pay a price in term of time wasted populating those files, and when we add the time collectively spent on those, I cannot imagine the few people's convenience would be worth that much effort. I vote for we stop using those ChangeLog files right now. We can still have them as part of the ChangeLog if people want -- although, I am also open to considering the pros and cons of keeping it, but I think they might not be worth the investiment either.
On 2018-02-02 10:53, Joel Brobecker wrote: > Well, if it were me, I would stop using ChangeLog files with > immediate effect. > > The purpose of ChangeLog files, as I understand it, is to describe > the "what changed", and is aimed at people who download a given > release and want to know what changed (because they do not have > access to the version control information). > > While the above might have been essential 20 or 25 years ago, > it is trivial for anyone today to have access to the change > history of our program. Two options: > - git clone our public repository; or... > - ... if they don't want to use git, they can use the gitweb > interface. > > I think that this is an acceptable way for us to provide the log > of changes that were made. > > Currently, the situation is that we all pay a price in term of time > wasted populating those files, and when we add the time collectively > spent on those, I cannot imagine the few people's convenience would > be worth that much effort. > > I vote for we stop using those ChangeLog files right now. We can > still have them as part of the ChangeLog if people want -- although, > I am also open to considering the pros and cons of keeping it, > but I think they might not be worth the investiment either. I agree about the time consumed producing ChangeLog entries. I have to admit that it sometimes discourages me to do structural changes that would affect a lot files/functions. I also feel like I'm filling some administrative paperwork, which is not my favorite way of spending my time. Maintaining an internal port of GDB, one use I have for the ChangeLog is to find what commit removed a given function. Let's say I rebase our code and find out a function we use was removed, grepping for that function would find the ChangeLog entry, leading me to the commit, which helps in fixing the code. However, I recently discovered "git blame --reverse", which allows to ask git about which commit removed a particular line. So the ChangeLog is not really required for that use case. Since with git the whole history is available offline, I think the usefulness of this kind of ChangeLogs is not as big as it used to be. To be fair I'll also state a pro I see for the ChangeLog. When I write the ChangeLog entry, it makes me look at my change a last time in details and sometimes find some additional issues I might not have found otherwise. It might be worth starting a new thread to get the attention of everybody to discuss this issue. Simon
On Fri, 2 Feb 2018, Joel Brobecker wrote: > > > We need a clear rule on writing changelog, otherwise how do we > > > write changelog or review patches? > > > > I agree. I hope Someone(TM) will answer these questions. > > Well, if it were me, I would stop using ChangeLog files with > immediate effect. Feel free to join the discussion on bug-standards (which has been going for several months) of eliminating the GNU Coding Standards requirement for the ChangeLog *format*. (It's already been allowed by the GCS for over 20 years to generate the ChangeLog *file* at release time for the release tarball from version control history rather than having it checked in, but then of course you need to make sure that commit messages are appropriately formatted to derive ChangeLog entries - so I consider that the format, not the file, is the main issue.)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 5c3338f..40f4008 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -144,23 +144,6 @@ 2018-01-27 Eli Zaretskii <eliz@gnu.org> - Avoid compilation errors in MinGW native builds - - The error is triggered by including python-internal.h, and the - error message is: - - In file included from d:\usr\lib\gcc\mingw32\6.3.0\include\c++\math.h:36:0, - from build-gnulib/import/math.h:27, - from d:/usr/Python26/include/pyport.h:235, - from d:/usr/Python26/include/Python.h:58, - from python/python-internal.h:94, - from python/py-arch.c:24: - d:\usr\lib\gcc\mingw32\6.3.0\include\c++\cmath:1157:11: error: '::hypot' has not been declared - using ::hypot; - ^~~~~ - - This happens because Python headers define 'hypot' to expand t - '_hypot' in the Windows builds. * python/python-internal.h (_hypot) [__MINGW32__]: Define back to 'hypoth'. This avoids a compilation error.