This is the mail archive of the
newlib@sourceware.org
mailing list for the newlib project.
RE: [Patch, MIPS] Update MIPS memcpy.S for mips32r6/mips64r6 support
- From: "Maciej W. Rozycki" <macro at linux-mips dot org>
- To: Matthew Fortune <Matthew dot Fortune at imgtec dot com>
- Cc: Steve Ellcey <Steve dot Ellcey at imgtec dot com>, "newlib at sourceware dot org" <newlib at sourceware dot org>
- Date: Tue, 24 Feb 2015 13:27:56 +0000 (GMT)
- Subject: RE: [Patch, MIPS] Update MIPS memcpy.S for mips32r6/mips64r6 support
- Authentication-results: sourceware.org; auth=none
- References: <2d3eb762-b10e-40bf-8f7f-57c85d9e1f7e at BAMAIL02 dot ba dot imgtec dot org> <6D39441BF12EF246A7ABCE6654B0235320FDE9BB at LEMAIL01 dot le dot imgtec dot org>
On Tue, 24 Feb 2015, Matthew Fortune wrote:
> Given Corinna's request for MIPS reviewers with my patches I
> expect we should co-review things for MIPS. I've been
> through and read the patch. I don't have any objection to
> this combined indentation and R6 patch especially given it
> is pretty much just making newlib match glibc.
FWIW, I think a semantical change that includes just a couple -- say two
or three lines' worth of -- formatting updates around the change proper
itself is fine, especially if the semantical changes are substantial and
difficult to miss.
Here the formatting updates considerably outweigh the change proper,
which IMHO obfuscates things both for the reviewer right now, and -- more
importantly -- for people having a need to look through repository
archives in the future.
So I think it would indeed make sense to split the change into two --
first that gives no output with `git diff -w' (or `git log -p -w', etc.),
and a follow-up change that actually adds mips32r6/mips64r6 support. And
for the record I'd say the same of a smaller change that reindented like
three lines containing complicated code and buried a variable reference
change from `v' to `w' between them. It would just be too easy to miss.
As I say, FWIW.
Maciej