Обсуждение: cleanup: Split long Makefile lists across lines and sort them
This makes sure that the lines don't get excessively long, while keeping diffs that add/remove elements small. For context: this was extracted from the pytest patchset and extended to more places based on Andres his recommendation.
Вложения
> This makes sure that the lines don't get excessively long, while keeping > diffs that add/remove elements small. Hi, It looks like there’s a minor typo in the patch: in the line `is first becaues`, `becaues` is a misspelling of `because`. -- Regards, Man Zeng www.openhalo.org
On Wed Dec 24, 2025 at 3:18 PM CET, =?gb18030?B?emVuZ21hbg==?= wrote: > It looks like there’s a minor typo in the patch: in the line `is first becaues`, > `becaues` is a misspelling of `because`. Oops. Fixed in attached.
Вложения
On Wed Dec 24, 2025 at 3:31 PM CET, Jelte Fennema-Nio wrote: > Oops. Fixed in attached. I had accidentally included a trailing backspace in one of the lists. To my surprise that worked fine. Since these trailing separators result in smaller git diffs when a new item is added at the end of the list, I decided to put such trailing backslashes in all the lists in this patch.
Вложения
No further comments from me, LGTM! -- Regards, Man Zeng www.openhalo.org
> On Dec 25, 2025, at 17:07, Jelte Fennema-Nio <postgres@jeltef.nl> wrote: > > On Wed Dec 24, 2025 at 3:31 PM CET, Jelte Fennema-Nio wrote: >> Oops. Fixed in attached. > > I had accidentally included a trailing backspace in one of the lists. To > my surprise that worked fine. Since these trailing separators result in > smaller git diffs when a new item is added at the end of the list, I > decided to put such trailing backslashes in all the lists in this patch. > <v3-0001-cleanup-Split-long-Makefile-lists-across-lines-an.patch> ``` +REGRESS = \ + page \ + brin \ + btree \ + checksum \ + gin \ + gist \ + hash \ + oldextversions \ ``` If we look at existing Makefiles, they don’t add a tailing “\” in the last line of lists, so let’s keep consistent. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
On Thu, 25 Dec 2025 at 10:07, "Jelte Fennema-Nio" <postgres@jeltef.nl> wrote: > On Wed Dec 24, 2025 at 3:31 PM CET, Jelte Fennema-Nio wrote: >> Oops. Fixed in attached. > > I had accidentally included a trailing backspace in one of the lists. To > my surprise that worked fine. Since these trailing separators result in > smaller git diffs when a new item is added at the end of the list, I > decided to put such trailing backslashes in all the lists in this patch. > [3. text/x-patch; v3-0001-cleanup-Split-long-Makefile-lists-across-lines-an.patch]... Small suggestion: the backslash on the last line isn't needed and can be removed. This also aligns with the style used elsewhere. -- Regards, Japin Li ChengDu WenWu Information Technology Co., Ltd.
On Thu Dec 25, 2025 at 10:37 AM CET, Chao Li wrote: > If we look at existing Makefiles, they don’t add a tailing “\” in the last line of lists, so let’s keep consistent. Alright, attached is one without the trailing backslash. If we want to add those let's indeed do it in a dedicated patch that makes all of such lists consistently use them.
Вложения
On Fri, Dec 26, 2025 at 06:30:53PM +0100, Jelte Fennema-Nio wrote: > On Thu Dec 25, 2025 at 10:37 AM CET, Chao Li wrote: > > If we look at existing Makefiles, they don’t add a tailing “\” in the last line of lists, so let’s keep consistent. > > Alright, attached is one without the trailing backslash. If we want to > add those let's indeed do it in a dedicated patch that makes all of such > lists consistently use them. Yep, none of these lists should have a trailing backslash. If nobody objects, I'll get this one applied while double-checking the rest of the tree, not having these lists make the addition of new items always harder to parse. -- Michael
Вложения
On Sat, Dec 27, 2025 at 08:30:59AM +0900, Michael Paquier wrote: > On Fri, Dec 26, 2025 at 06:30:53PM +0100, Jelte Fennema-Nio wrote: >> > If we look at existing Makefiles, they don’t add a tailing “\” in the last line of lists, so let’s keep consistent. >> >> Alright, attached is one without the trailing backslash. If we want to >> add those let's indeed do it in a dedicated patch that makes all of such >> lists consistently use them. Expanded that to a few more SUBDIRS noticed on the way, without the backslash after the last item of each list, and applied the result. We have quite a few more REGRESS lists in contrib/. Would these be worth changing for test_decoding or pg_stat_statements at least? -- Michael
Вложения
On Sat, Dec 27, 2025 at 08:30:59AM +0900, Michael Paquier wrote:
> On Fri, Dec 26, 2025 at 06:30:53PM +0100, Jelte Fennema-Nio wrote:
>> > If we look at existing Makefiles, they don’t add a tailing “\” in the last line of lists, so let’s keep consistent.
>>
>> Alright, attached is one without the trailing backslash. If we want to
>> add those let's indeed do it in a dedicated patch that makes all of such
>> lists consistently use them.
Expanded that to a few more SUBDIRS noticed on the way, without the
backslash after the last item of each list, and applied the result.
We have quite a few more REGRESS lists in contrib/. Would these be
worth changing for test_decoding or pg_stat_statements at least?
> On Fri, Dec 26, 2025 at 06:30:53PM +0100, Jelte Fennema-Nio wrote:
>> > If we look at existing Makefiles, they don’t add a tailing “\” in the last line of lists, so let’s keep consistent.
>>
>> Alright, attached is one without the trailing backslash. If we want to
>> add those let's indeed do it in a dedicated patch that makes all of such
>> lists consistently use them.
Expanded that to a few more SUBDIRS noticed on the way, without the
backslash after the last item of each list, and applied the result.
We have quite a few more REGRESS lists in contrib/. Would these be
worth changing for test_decoding or pg_stat_statements at least?
I have a few questions:
1. Regarding splitting long lists: Is there an established convention or
threshold (e.g., maximum number of items per line or maximum line length)
that we should follow when deciding whether to break a list across multiple
lines?
2. I noticed that btree_gin, btree_gist, and pgcrypto also have relatively
long REGRESS lists, should we also update them?
3. Does this formatting guideline also extend to the DATA variable (for
example in contrib Makefiles), or is it mainly intended for REGRESS lists?
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
On Sun, Dec 28, 2025 at 04:12:16AM +0000, Japin Li wrote: > 2. I noticed that btree_gin, btree_gist, and pgcrypto also have relatively > long REGRESS lists, should we also update them? If we think about that in terms of frequency where new test files are added, it seems to me that there would be more meaning to do that for pgcrypto, test_decoding and pgss. This is more true for the two last entries of this list. About the rest? Not so much. Still, I was surprised to see that we have added stuff to btree_gist recently for WITHOUT OVERLAPS, so perhaps there is a point there. -- Michael
Вложения
> On Dec 29, 2025, at 07:29, Michael Paquier <michael@paquier.xyz> wrote: > > On Sun, Dec 28, 2025 at 04:12:16AM +0000, Japin Li wrote: >> 2. I noticed that btree_gin, btree_gist, and pgcrypto also have relatively >> long REGRESS lists, should we also update them? > > If we think about that in terms of frequency where new test files are > added, it seems to me that there would be more meaning to do that for > pgcrypto, test_decoding and pgss. This is more true for the two last > entries of this list. > > About the rest? Not so much. Still, I was surprised to see that we > have added stuff to btree_gist recently for WITHOUT OVERLAPS, so > perhaps there is a point there. > -- > Michael This patch reminded me an old technique of using ENDLIST to terminate mule-line lists in Makefile. I started a new threadfor that. Please see the details in [1]. [1] https://www.postgresql.org/message-id/CAEoWx2nsiH9PxBKtD_pVLLYVHUv8%3DFohczchT550a7VT%2BLZq4g%40mail.gmail.com Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Michael Paquier <michael@paquier.xyz> writes:
> On Sun, Dec 28, 2025 at 04:12:16AM +0000, Japin Li wrote:
>> 2. I noticed that btree_gin, btree_gist, and pgcrypto also have relatively
>> long REGRESS lists, should we also update them?
> If we think about that in terms of frequency where new test files are
> added, it seems to me that there would be more meaning to do that for
> pgcrypto, test_decoding and pgss. This is more true for the two last
> entries of this list.
> About the rest? Not so much. Still, I was surprised to see that we
> have added stuff to btree_gist recently for WITHOUT OVERLAPS, so
> perhaps there is a point there.
I think a reasonable policy for this going forward is to mop up
the stragglers when and if another test case is added to each one.
regards, tom lane