From: Pedro Alves <palves@redhat.com>
To: Simon Marchi <simon.marchi@polymtl.ca>, gdb-patches@sourceware.org
Subject: Re: [PATCH 3/4] Makefile: Flatten and sort file lists
Date: Tue, 15 Nov 2016 16:46:00 -0000 [thread overview]
Message-ID: <e554e659-bf65-e42f-a1ff-6da69c6e2fa6@redhat.com> (raw)
In-Reply-To: <20161113034625.8237-4-simon.marchi@polymtl.ca>
On 11/13/2016 03:46 AM, Simon Marchi wrote:
> I find the big file lists in the Makefile a bit ugly and not very
> practical. Since there are multiple filenames on each line (as much as
> fits in 80 columns), it's not easy to add, remove or change a name in
> the middle. As a result, we have a mix of long and short lines in no
> particular order (ALL_TARGET_OBS is a good example).
>
> I therefore suggest flattening the list (one name per line) and keeping
> it in alphabetical order. The diffs will be much clearer and merge
> conflicts will be easier to resolve.
I agree. I've asked for similar things in other cases for the same
reason.
>
> For the ordering, I aimed at respecting this order:
>
> * foo.c
> * foo-bar.c
> * foobar.c
>
> Which, in non-formal words, could be expressed like so:
>
> * The extension is not taken into account when comparing two filenames
> (except if the filenames are otherwise equal).
> * A filename that is a prefix of another one comes before this other
> one.
> * Underscores and dashes are treated equally (I think we use them both
> for the same purpose).
> * Underscores and dashes come before alphanumeric characters.
>
> Short of any other rule, this could be used as a guideline to determine
> where to add a new filename in the list.
Could/should this be converted into a comment? At least a "keep these
lists sorted" comment, I think. That'll be easier to find than this email
or commit log.
Wouldn't it make sense to sort by files and dirs separately? I.e.,
files first, then dirs, e.g.? You have this, for example:
> + mdebugread.h \
> + memattr.h \
> + memory-map.h \
> + memrange.h \
> + mi/mi-cmds.h \
> + mi/mi-common.h \
> + mi/mi-console.h \
> + mi/mi-getopt.h \
> + mi/mi-main.h \
> + mi/mi-out.h \
> + mi/mi-parse.h \
> + microblaze-tdep.h \
> + mips-linux-tdep.h \
> + mips-tdep.h \
> + mipsnbsd-tdep.h \
I.e., MI files mixed with the other files. I'd expected:
> + mdebugread.h \
> + memattr.h \
> + memory-map.h \
> + memrange.h \
> + microblaze-tdep.h \
> + mips-linux-tdep.h \
> + mips-tdep.h \
> + mipsnbsd-tdep.h \
...
> + mi/mi-cmds.h \
> + mi/mi-common.h \
> + mi/mi-console.h \
> + mi/mi-getopt.h \
> + mi/mi-main.h \
> + mi/mi-out.h \
> + mi/mi-parse.h \
Maybe it's be clearer to move subdir headers to
variables, like we have SUBDIR_PYTHON_SRCS/SUBDIR_PYTHON_OBS,
etc. for sources. But still, for the cases where we don't have
variables.
> SUBDIR_TUI_OBS = \
> + tui.o \
> tui-command.o \
> tui-data.o \
> tui-disasm.o \
> @@ -269,9 +304,9 @@ SUBDIR_TUI_OBS = \
> tui-windata.o \
> tui-wingeneral.o \
> tui-winsource.o \
> - tui.o
Mind the trailing \ left in the last element. Here
and other places.
Thanks,
Pedro Alves
next prev parent reply other threads:[~2016-11-15 16:46 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-13 3:46 [PATCH 0/4] Makefile improvements and cleanups Simon Marchi
2016-11-13 3:46 ` [PATCH 1/4] Makefile: Replace old suffix rules with pattern rules Simon Marchi
2016-11-13 3:46 ` [PATCH 2/4] Makefile: Replace explicit subdir " Simon Marchi
2016-11-15 17:54 ` Pedro Alves
2016-11-15 18:00 ` Simon Marchi
2016-11-15 18:36 ` Pedro Alves
2016-11-15 18:41 ` Simon Marchi
2016-11-13 3:46 ` [PATCH 3/4] Makefile: Flatten and sort file lists Simon Marchi
2016-11-15 16:46 ` Pedro Alves [this message]
2016-11-13 3:56 ` [PATCH 0/4] Makefile improvements and cleanups Simon Marchi
2016-11-13 8:49 ` Andreas Schwab
2016-11-13 15:26 ` Simon Marchi
2016-11-13 16:25 ` Eli Zaretskii
2016-11-15 16:57 ` Pedro Alves
2016-11-15 17:15 ` Simon Marchi
2016-11-15 17:51 ` Pedro Alves
2016-11-15 21:58 ` Pedro Alves
2016-11-15 22:10 ` Simon Marchi
2016-11-15 21:46 ` John Baldwin
2016-11-17 18:54 ` John Baldwin
2016-11-17 19:20 ` Simon Marchi
2016-11-17 22:36 ` John Baldwin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e554e659-bf65-e42f-a1ff-6da69c6e2fa6@redhat.com \
--to=palves@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=simon.marchi@polymtl.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox